-
Notifications
You must be signed in to change notification settings - Fork 251
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
treat collections in packages as immutable #663
Conversation
I tend to say it's worth it.
Maybe, we should add a comment in the Is there a reason why you left |
both of your suggestions sound reasonable: if you're active and interested then feel free to make updates directly else I'll try to get to it - but likely not for at least a few days |
Kudos, SonarCloud Quality Gate passed! |
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 transparency: I'm planning to merge this shortly before the core release because I think merging it now without a release will make development on other Poetry PRs that require core changes more difficult.
5d019df
to
4d4d75e
Compare
justifying the use of a shallow copy in Package.clone()
4d4d75e
to
a281c5b
Compare
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
intended as a more-or-less maintainable version of #618. See also python-poetry/poetry#8671
by annotating collection types in packages as immutable, we "prove" that the shallow copy is a safe way to clone them
of course this is still not too hard to break: just by adding in something mutable later.
I see performance improvement during locking by about 20-25% on a couple of sample projects. It's not a complete no-brainer to me that this is change is safe enough to accept, but I think it probably is: and that's a good amount of performance to leave on the table. Still, I'm ready to be overruled.