-
Notifications
You must be signed in to change notification settings - Fork 310
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
Avoid unzipping .nupkg every time we need to read from .nuspec by saving .nuspec inside the package folder instead of .nupkg #570
Conversation
…d unzipping to read nuspec. Managed backwards compatibility and fixed tests
Hi, |
src/NuGetForUnity/Editor/PackageSource/NugetPackageSourceLocal.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 rebase, and there is one missing conversation
After this gets approved, I will push the squashed commit directly to master. |
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.
@popara96 why directly pushing to master. I don't like when PRs are skipped as they ensure e.g. that the CI works. I would just squash the commits -> rebase -> merge PR. So we keep the link to the PR / it will be listed correctly inside the Release Nodes.
Does that mean we force push the branch after rebase + squash? We were not sure if we force push was allowed. |
Yes force-push on branches is allowed (not on main), you can do --force-with-leas so the complete history still exists. At leas I do it, but I always create branches on my fork. Without force-push it would be also possible but then you need to do a merge commit (I also prefer rebase over merging) |
Backwards compatibility is managed by checking for .nupkg files and if there are some, we unpack .nuspec file from them and delete them. Tests have also been changed accordingly.