-
Notifications
You must be signed in to change notification settings - Fork 688
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
error when attempting to pack multiple files into the same destination #3923
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.
Maybe bit perf improvement suggestion.
src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs
Outdated
Show resolved
Hide resolved
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.
🎈 for Validation!
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 for me.
2264429
to
8de50a6
Compare
src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs
Outdated
Show resolved
Hide resolved
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.
We need to account for the case sensitivity and I think @kartheekp-ms has some good feedback as well.
Bug
Fixes: NuGet/Home#6941
Regression? Last working version:
Description
This PR makes it so that pack fails if you try to pack two or more files into the exact same destination. We believe this is acceptable because, even though this would be a new error, the packages generated when this was happening are pretty much broken.
PR Checklist
PR has a meaningful title
PR has a linked issue.
Described changes
Tests
Documentation