-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Suggest =>
--> >=
in comparisons
#117303
Suggest =>
--> >=
in comparisons
#117303
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @wesleywiser (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
This comment has been minimized.
This comment has been minimized.
I don't think this handles cases where the condition is chained with |
There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged. You can start a rebase with the following commands:
The following commits are merge commits: |
This comment has been minimized.
This comment has been minimized.
1c43556
to
1b53936
Compare
Thanks for the feedback! Chaining with |
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #117769) made this pull request unmergeable. Please resolve the merge conflicts. |
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.
Please squash rebase on top of a recent master.
ad088ef
to
725a73c
Compare
This comment has been minimized.
This comment has been minimized.
I tried to rebase and bless the tests for you, but there seems to be something wrong still:
looks like it doesn't apply the fix properly for all the cases, only for some. |
725a73c
to
97cf1c8
Compare
Thanks for the help, and sorry for dragging my feet on this--things have been keeping me pretty busy for the past few weeks. I've updated the recovery code + test output to catch more cases while hopefully keeping false positives low. @rustbot review |
☀️ Test successful - checks-actions |
Finished benchmarking commit (a861c89): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 670.827s -> 670.98s (0.02%) |
Fixes #117245