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.

Sign up for GitHub

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

Jump to bottom

[repo/examples] Prepare to .NET9 #2251

Merged
merged 6 commits into from
Nov 25, 2024
Merged

[repo/examples] Prepare to .NET9 #2251

merged 6 commits into from
Nov 25, 2024

Conversation

Kielek
Copy link
Contributor

@Kielek Kielek commented Oct 28, 2024

.NET 9 brings new set of analyzers.
Analyzers are enabled

opentelemetry-dotnet-contrib/build/Common.props

Line 13 in cf14a5a

<AnalysisLevel>latest-all</AnalysisLevel>
and will be included in pipeline when .NET SDK is updated.

Changes

Fix new warnings by building projects locally with .NET9 SDK RC2

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
  • [ ] Unit tests added/updated
  • [ ] Appropriate CHANGELOG.md files updated for non-trivial changes
  • [ ] Changes in public API reviewed (if applicable)

@Kielek Kielek requested a review from a team as a code owner October 28, 2024 05:43
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Oct 28, 2024
@codecov Codecov
Copy link

codecov bot commented Oct 28, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.74%. Comparing base (71655ce) to head (1d3cad9).
Report is 615 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #2251       +/-   ##
===========================================
- Coverage   73.91%   61.74%   -12.18%     
===========================================
  Files         267       73      -194     
  Lines        9615     2044     -7571     
===========================================
- Hits         7107     1262     -5845     
+ Misses       2508      782     -1726
Flag Coverage Δ
unittests-Instrumentation.AspNet 77.00% <ø> (?)
unittests-Instrumentation.ConfluentKafka 14.37% <ø> (?)
unittests-Instrumentation.Owin 88.12% <ø> (?)
unittests-Instrumentation.Process 100.00% <ø> (?)
unittests-Instrumentation.Runtime 100.00% <ø> (?)
unittests-Instrumentation.Wcf 78.57% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 291 files with indirect coverage changes

@Kielek Kielek mentioned this pull request Oct 28, 2024
Merged
1 task
@Kielek Kielek marked this pull request as draft October 28, 2024 06:17
This was referenced Oct 28, 2024
Merged
Merged
Merged
CodeBlanch
CodeBlanch reviewed Oct 28, 2024
View reviewed changes
examples/grpc.core/Examples.GrpcCore.AspNetCore/Controllers/WeatherForecastController.cs Outdated
@@ -7,12 +7,12 @@ namespace Examples.GrpcCore.AspNetCore.Controllers;

[ApiController]
[Route("[controller]")]
public class WeatherForecastController : ControllerBase
internal class WeatherForecastController : ControllerBase
Copy link
Member

Choose a reason for hiding this comment

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

I just tested this. AspNetCore won't auto-register internal controller so this effectively breaks the app. Curious, what is the warning you are getting spurring this change?

Copy link
Contributor Author

@Kielek Kielek Oct 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

dotnet format opentelemetry-dotnet-contrib.sln when executing in .NET9 SDK context (I just remove global.json file on my env).

It was one of the reasons to mark is as in progress - the second one, some differences in formatting between .NET8 (CI) and .NET9.

This was referenced Oct 29, 2024
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
This was referenced Nov 4, 2024
Merged
Merged
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 5, 2024

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Nov 5, 2024
This was referenced Nov 5, 2024
Merged
Merged
Merged
Merged
@Kielek Kielek removed the Stale label Nov 8, 2024
@xiang17 xiang17 mentioned this pull request Nov 12, 2024
Merged
2 tasks
@Kielek Kielek mentioned this pull request Nov 12, 2024
Open
4 tasks
@xiang17 xiang17 mentioned this pull request Nov 13, 2024
Open
@Kielek Kielek mentioned this pull request Nov 14, 2024
Open
@github-actions GitHub Actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Nov 16, 2024
@Kielek Kielek removed the Stale label Nov 20, 2024
This was referenced Nov 21, 2024
Merged
Merged
@github-actions github-actions bot added the infra Infra work - CI/CD, code coverage, linters label Nov 25, 2024
@github-actions github-actions bot removed the infra Infra work - CI/CD, code coverage, linters label Nov 25, 2024
@Kielek Kielek marked this pull request as ready for review November 25, 2024 11:21
CodeBlanch
CodeBlanch approved these changes Nov 25, 2024
View reviewed changes
Copy link
Member

@CodeBlanch CodeBlanch left a comment

Choose a reason for hiding this comment

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

LGTM

@Kielek Kielek merged commit 65629b0 into open-telemetry:main Nov 25, 2024
73 checks passed
@Kielek Kielek deleted the net9prep-examples branch November 25, 2024 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CodeBlanch CodeBlanch CodeBlanch approved these changes

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Kielek @CodeBlanch