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

.NET9 support #3796

Merged
merged 29 commits into from
Nov 21, 2024
Merged

.NET9 support #3796

merged 29 commits into from
Nov 21, 2024

Conversation

Kielek
Copy link
Contributor

@Kielek Kielek commented Nov 20, 2024

Why

Towards #3766

What

Adds support for .NET9.
Drops support for .NET6 and .NET7.
Other related cleanup.
Bumping .NET SDK to 8.0.404
LangVersion updated to 13 (version supported by .net 9). previous version was taken from .NET7 (needed for some genrated, grpc code).

Tests

CI

Checklist

  • CHANGELOG.md is updated.
  • Documentation is updated.
  • New features are covered by tests.

Notes

dotnet format is failing. Not related to changes.
PR is huge, IMO worth to review commit by commit.

@Kielek
Copy link
Contributor Author

Kielek commented Nov 20, 2024

@stevejgordon, fyi

@Kielek Kielek marked this pull request as ready for review November 20, 2024 13:05
@Kielek Kielek requested a review from a team as a code owner November 20, 2024 13:05
Copy link
Contributor

@zacharycmontoya zacharycmontoya left a comment

Choose a reason for hiding this comment

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

I've looked through and overall it looks good. I'll wait for resolution on existing comments before approving

Copy link
Member

@nrcventura nrcventura left a comment

Choose a reason for hiding this comment

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

There are many places in the code where we needed to change NET60_OR_GREATER to NET80_OR_GREATER. It seems like the intent of the directive in many, if not most of those locations was to use a version of the code that only applied to the .net build target and not the .net framework build target.

Where appropriate, should we change NET80_OR_GREATER to either NET or !NETFRAMEWORK, to make the intent clearer?

@Kielek
Copy link
Contributor Author

Kielek commented Nov 21, 2024

There are many places in the code where we needed to change NET60_OR_GREATER to NET80_OR_GREATER. It seems like the intent of the directive in many, if not most of those locations was to use a version of the code that only applied to the .net build target and not the .net framework build target.

Where appropriate, should we change NET80_OR_GREATER to either NET or !NETFRAMEWORK, to make the intent clearer?

@nrcventura, I agree, but I do not want to make such changes in the same file. just kept the current convention. New issue created: #3798 (not a blocker to the release IMO, just normal maintenance task).

Copy link
Contributor

@RassK RassK left a comment

Choose a reason for hiding this comment

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

Some minor things and tweaks only. In overall tests are passing, seems like good to go.

Copy link
Contributor

@zacharycmontoya zacharycmontoya left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@Kielek Kielek merged commit e205769 into open-telemetry:main Nov 21, 2024
38 of 39 checks passed
@Kielek Kielek deleted the net9 branch November 21, 2024 19:23
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.

4 participants