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

Create global.json file #16560

Merged
merged 1 commit into from
Aug 15, 2024
Merged

Create global.json file #16560

merged 1 commit into from
Aug 15, 2024

Conversation

sebastienros
Copy link
Member

Ensures we are using an 8.0.x SDK when building the repos. If you have a newer version some errors are not triggered and other warnings are displayed.

@@ -0,0 +1,6 @@
{
"sdk": {
"version": "8.0.101",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"version": "8.0.101",
"version": "8.0.400",

Copy link
Member Author

Choose a reason for hiding this comment

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

That as on purpose not to "force" users to have a newer version. And the rollForward will still take the newer version if you have it.

This way also we don't have to update the file with every single new version.
We could pick another more recent version too if we have a reason, like high security patch that we require users to use (why would "we" force them though).

Copy link
Member

@Piedone Piedone left a comment

Choose a reason for hiding this comment

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

This is good but something that we'll miss when upgrading between major .NET versions, like to .NET 10. I also don't know how we'd handle this when double-targeting .NET 8 and 9. So, would you please add a docs page with a checklist on what we'll have to do then?

Apart from this file, the other thing I can think of is replacing all net8.0 instances. But there were more I don't remember. This gets increasingly complicated and we'll forget things.

Also see:

@sebastienros
Copy link
Member Author

When we need to support a higher version we jump bump the sdk version to a compatible one.

For instance when we create the PR to support 9 (and 8) then we also bump this file to a 9.0 sdk version.

This PR is just to ensure we don't use a 9.0-preview version today, when it's on a local box, because it won't build (new warnings from the new SDK that fail the build). But when we do the work on supporting net9 then the warnings will be fixed.

@Piedone
Copy link
Member

Piedone commented Aug 15, 2024

Great, now please copy this to a docs page :). Whoever will do this next time won't necessary know about this.

@MikeAlhayek
Copy link
Member

@sebastienros should be merge #16567 and close this PR?

@sebastienros sebastienros merged commit 01725b2 into main Aug 15, 2024
32 checks passed
@sebastienros sebastienros deleted the sebros/globaljson branch August 15, 2024 18:13
@Piedone
Copy link
Member

Piedone commented Aug 15, 2024

Kinda defeats the purpose of a review if you merge against the feedback ;).

@sebastienros
Copy link
Member Author

Nobody understood what kind of doc was necessary. It's just ensuring it's using 8 sdk instead of a newer one that could break local builds.

@Piedone
Copy link
Member

Piedone commented Aug 15, 2024

When doing the nest .NET update, this is one more thing to not forget to take care of. It's easier to follow a checklist and update the file rather than a) remember to do it (including those people who haven't seen this conversation, to begin with) or b) figure out that you have to update it after the build fails with some error.

@sebastienros
Copy link
Member Author

I don't think anyone that doesn't know what global.json is should try to update OC to net9.0.
If we already have a doc to upgrade to net9.0, though this should be straightforward, I'll add a line to check the file. If we don't have a file, when the PR is created we'll decide if the change require a dedicated doc.

We may also add a comment in the csproj where the default tfms are defined.

@Piedone
Copy link
Member

Piedone commented Aug 15, 2024

It's not that you don't know what a global.json is. It's that you have know that OC has one, and remember to update it, or otherwise get bumped around by build errors when you forgot. It's not rocket science, but there are a dozen such non-rocket science things to do, and since we do it once a year, so nobody has much of an experience.

But I see under #16573 that you started to do this, great!

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.

5 participants