-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Clarify news entry for --trim #58992
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
base: release-1.12
Are you sure you want to change the base?
Conversation
b538d06
to
6800d81
Compare
6800d81
to
1de2eb9
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.
The change is incorrect. The trim feature works as stated, not as proposed
@Keno in that case maybe it shoould be rephrased as "only keeps code that is provably reachable from the entry point"? |
Removing the double negative is is fine as long as it doesn't change the semantics. Notably "proven" and "provable" are different concepts. |
Thanks! I changed the language based on the language used in the trim PR #55047 |
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 this is now correct but defer to @Keno for a final review.
I will note that the word "statically" was explicitly discouraged here #56460 (comment) |
d9634e4
to
c71f1cf
Compare
fixed, thanks! |
@ericphanson Thanks, fixed |
This wording is fine but these are very annoying to backport since the change has to be moved to a separate file. |
The news entry for
--trim
in release 1.12 uses a double negative which is a little confusing. This PR fixes thatEDIT: added description