Uh oh!
There was an error while loading. Please reload this page.
[breaking batch] remove the sign from integer literals in the ast - #30508
[breaking batch] remove the sign from integer literals in the ast#30508oli-obk wants to merge 2 commits into
Conversation
rust-highfive
commented
Dec 21, 2015
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @eddyb (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
There was a problem hiding this comment.
Shouldn't lit be returned early if the value is positive?
eddyb
commented
Dec 21, 2015
LGTM. r? @Manishearth for confirmal of acceptable breakage. |
Manishearth
commented
Dec 21, 2015
This will likely cause aster breakage. I'm mostly okay with the clippy breakage this may cause (there's not much using this) |
Manishearth
commented
Dec 22, 2015
nrc
commented
Dec 22, 2015
What's the motivation for this work? Seems like a good change to make to the HIR, not sure if it is a good change for the AST - does this conflate different source situations? ( At the least, this should be batched with other changes. |
eddyb
commented
Dec 22, 2015
@nrc Rust doesn't have |
oli-obk
commented
Dec 22, 2015
The only way to get a |
nrc
commented
Dec 22, 2015
@eddyb as in, the |
nrc
commented
Dec 22, 2015
In which case, this sounds like a good change, but I'd still like to see it batched with other changes since it is non-urgent. |
Manishearth
commented
Dec 23, 2015
Should we add some kind of github label for these PRs so it's easy to find them when we need to batch up? plugins-breaking or something |
45cd3c4 to
2bb0d02Comparebors
commented
Jan 12, 2016
☔ The latest upstream changes (presumably #30826) made this pull request unmergeable. Please resolve the merge conflicts. |
pnkfelix
commented
Jan 12, 2016
I vote for "breaking-batch" (Apologies to Walter White) |
steveklabnik
commented
Feb 8, 2016
This has been rebased, is it ready to go? |
Manishearth
commented
Feb 8, 2016
@oli-obk want to uplift this to the mega-breaking-batch PR? Since that will probably bitrot this when it lands. |
oli-obk
commented
Feb 9, 2016
already done |
r? @eddyb