-
Notifications
You must be signed in to change notification settings - Fork 702
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
Update PackageUpdateResource to receive bool to indicate if snupkg are enabled #6249
base: dev
Are you sure you want to change the base?
Conversation
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.
Looks good, but we need tests.
I see you've crossed off the added tests part but there's no justification for missing tests.
https://github.com/NuGet/NuGet.Client/blob/dev/test/NuGet.Clients.FuncTests/NuGet.CommandLine.FuncTest/Commands/PushCommandTest.cs has some tests.
Ideally, we can consider adding these tests to dotnet nuget push
instead.
The setup code should be very similar.
445655c
to
4f0fcc2
Compare
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 had left some comments, but didn't click submit :D
Bug
Fixes: NuGet/Home#11871
Description
SymbolPackageUpdateResource is currently only being used in PackageUpdateResource to determine if snupkg are enabled for symbol sources. Since the push command allows users to specify a SymbolSource, there may not be a SymbolPackageUpdateResource available. This created a new function which takes in "allowSnupkg" as a parameter instead of trying to determine if it's available based on whether or not SymbolPackageUpdateResource is null.
PR Checklist
Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.