Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[main] Upgrade netcoreapp3.1 TFMs to net7.0 #9127
[main] Upgrade netcoreapp3.1 TFMs to net7.0 #9127
Changes from 15 commits
510f582
9d4bd2b
bc91aae
b8236d1
49e11af
1db4d43
0d9dcdc
b65afc0
f6af788
b88968b
f87cc56
d46523a
ef51e04
d262b23
08c7848
2e1ec30
3350777
3448629
9627a60
5200224
a881adf
a39a29f
9073eb7
2a9d198
d0090ab
b5794ff
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Unsure who owns nugetrepack. @mmitche this will probably not work until microsoft.dotnet.nugetrepack.tasks is updated after this PR is merged and the newer package is available.
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.
Also it's weird that something outside the package hardcodes the path to the assemblies inside the package. Ideally the microsoft.dotnet.nugetrepack.tasks package would contain an msbuild file that already sets this property (as we do in all our other task projects in arcade).
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.
@ViktorHofer Not sure who owns nuget repack either. My guess with the properties is just that this is an older-style and an oversight,
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.
I tested this change in fsharp, which I know uses NuGetRepack, and it does work. See here for my build of fsharp (internal MS link): https://dev.azure.com/dnceng/internal/_build/results?buildId=1829578&view=results. The windows failure is an unrelated, known issue: https://github.com/microsoft/dropvalidator/issues/455
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.
Yes this should work. My comment was referring to that ideally this code would reside in the package itself and not in the consumer's path. We should look at this in a follow-up.
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.
I think that this property and the target at the bottom of the project file aren't required anymore based on the underlying msbuild issue already being fixed. That said, this might be a good follow-up investigation, unrelated to this PR.
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.
I tried this change even with the preview 5 .NET 7 SDK and it still cant load 7.0.0 of System.Runtime for this task.
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.
@AraHaan can you please post or message to me privately a link to the failure you're describing?