-
Notifications
You must be signed in to change notification settings - Fork 13
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
Initial changes for .NET 9 RPMs #345
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,7 @@ IFS=$'\n\t' | |
set -x | ||
|
||
sdk_version="$(dotnet --version)" | ||
runtime_version="$(dotnet --list-runtimes | head -1 | awk '{ print $2 }')" | ||
runtime_version="$(dotnet --list-runtimes | grep Microsoft.NETCore.App | awk '{ print $2 }')" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
runtime_id=$(../runtime-id) | ||
# This might be the final/only netstandard version from now on | ||
netstandard_version=2.1 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,6 @@ else | |
fi | ||
|
||
framework_dir=$(../dotnet-directory --framework "$1") | ||
dotnet tool update -g dotnet-ef --version "${framework_dir#*App/}.*-*" | ||
dotnet tool update -g dotnet-ef --version "${framework_dir#*App/}.*" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure why we previously used There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With a preview version, this was evaluated as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nicrowe00 updated this test since it was failing for .NET 8.0.1 here: #333. This test is skipped in our CI, so we haven't noticed it doesn't work against preview versions. With the March release ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should I merge this change, then, or defer it to a separate PR (or never)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nicrowe00 can you remind us why we had to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
He didn't add it. It was already there. Before #333 we prefixed this only with the major version number. So the With #333, if you run against a released version of .NET, you get something like When #333 runs against a .NET 9 preview, the This PR is now changing that version into
I'm fine with either what you have here, or the pattern we had before #333 (which will work for .NET 9 previews, and should start working for .NET 8 on the next patch release). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I was thinking of #319. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because I suggested it in that PR for the reasons mentioned above. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the documentation for the wildcard pattern: https://github.com/NuGet/Home/wiki/Support-pre-release-packages-with-floating-versions. |
||
|
||
dotnet ef --version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RPMs previously used
preview.1
instead ofpreview.1.24080.9
mostly because it was difficult to get accurate information before building things. With release.json, this is easy to fix, so I fixed it.