Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: rework CI pipeline to support C# 12, .NET 8.0 and tests on .NET Framework 4.8 #613

Conversation

Sylvain2703
Copy link

@Sylvain2703 Sylvain2703 commented Feb 3, 2024

Proposed Changes

Features

  • Complete rework of the CI pipeline to build with the latest .NET SDK and support C# 12:
  • Add support for .NET 8.0 and .NET Framework 4.8 on test projects.

Tests

  • Adjust tests to pass on Windows.

Build

  • Move ExampleConsole project into a subfolder to avoid it being in the same folder as ExampleBlazor.
  • Remove warnings for out of support target frameworks in test and example projects.

Checklist

  • CHANGELOG.md updated
  • Rebased/mergeable
  • A test has been added if appropriate
  • dotnet test completes successfully
  • Commit messages are conventional
  • Sign CLA (if not already signed)

…t .NET SDK

The .NET solution is now built and packed with a .NET 8 SDK Docker image and the resulting build is copied for testing with .NET Core 3.1/5/6/7/8 and for deployment.

Install InfluxDB 2.x to perform all tests on Windows (only Client.Legacy was tested).

Simplify some steps.
@codecov-commenter
Copy link

codecov-commenter commented Feb 3, 2024

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (9c54998) 87.13% compared to head (5d97ebf) 89.26%.
Report is 87 commits behind head on master.

Files Patch % Lines
Client.Core/Internal/AbstractQueryClient.cs 60.00% 3 Missing and 1 partial ⚠️
Client.Core/Internal/EnumConverter.cs 0.00% 2 Missing ⚠️
Client/Internal/ApiClient.cs 83.33% 2 Missing ⚠️
Client/Internal/MeasurementMapper.cs 0.00% 2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #613      +/-   ##
==========================================
+ Coverage   87.13%   89.26%   +2.12%     
==========================================
  Files          77       47      -30     
  Lines        6893     2123    -4770     
  Branches        0      311     +311     
==========================================
- Hits         6006     1895    -4111     
+ Misses        887      176     -711     
- Partials        0       52      +52     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- dotnet-6.0
- dotnet-7.0
- dotnet-windows
# - check-code-formatting # TODO: uncomment
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code formatting check is currently commented as many new C# features (introduced after C# 8) are applied by ReSharper (eg. file scoped namespaces, target-typed new expressions).

I see two options:

  • A big commit that reformats all files,
  • Override some unnecessary formatting rules (like file scoped namespaces).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can leave it commented out for now and, after merging this PR, create another one to address the formatting issues. Does this approach work for you?

key: *cache-key
paths:
- ./ReSharperCLI

deploy-preview:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you deploy production NuGet packages (those without -dev.CIRCLE_BUILD_NUM)?
We can probably integrate this deployment phase in this pipeline with a manual approval.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to deploy production packages by CI.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, that's a shame because it would keep track of build and test logs, and would also guarantee that all tests are carried out on the exact same assemblies deployed in production...

@Sylvain2703 Sylvain2703 changed the title feat: rework CI pipeline to support for C# 12, .NET 8.0 and tests on .NET Framework 4.8 feat: rework CI pipeline to support C# 12, .NET 8.0 and tests on .NET Framework 4.8 Feb 4, 2024
Copy link
Contributor

@bednar bednar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR 👍. After first quick look there is a few requirements to this change:

  1. Add README.md into Examples/ folder with link to console and Blazer example and also:

Client.Linq.Test/InfluxDBQueryVisitorTest.cs Show resolved Hide resolved
Client.Linq.Test/InfluxDBQueryVisitorTest.cs Show resolved Hide resolved
Examples/ExampleConsole/README.md Show resolved Hide resolved
Client.Linq.Test/InfluxDBQueryVisitorTest.cs Show resolved Hide resolved
Client.Linq.Test/InfluxDBQueryVisitorTest.cs Show resolved Hide resolved
dotnet nuget push ./NuGetPackages/InfluxDB.Client.*.nupkg -s ${BONITOO_NUGET_URL} -k ${BONITOO_SNAPSHOT_APIKEY} -sk ${BONITOO_SNAPSHOT_APIKEY}
Copy link
Contributor

@bednar bednar Feb 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please ensure to include dev.$CIRCLE_BUILD_NUM. as part of the regex. This is crucial to guarantee that only the packages generated in the current CI run are pushed.

Suggested change
dotnet nuget push ./NuGetPackages/InfluxDB.Client.*.nupkg -s ${BONITOO_NUGET_URL} -k ${BONITOO_SNAPSHOT_APIKEY} -sk ${BONITOO_SNAPSHOT_APIKEY}
dotnet nuget push ./NuGetPackages/InfluxDB.Client.*-dev.$CIRCLE_BUILD_NUM.nupkg -s ${BONITOO_NUGET_URL} -k ${BONITOO_SNAPSHOT_APIKEY} -sk ${BONITOO_SNAPSHOT_APIKEY}

@bednar
Copy link
Contributor

bednar commented Aug 5, 2024

This PR has been closed because it has not had recent activity. Please reopen if this PR is still important to you and you want to continue with them.

@bednar bednar closed this Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants