Skip to content
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

Use bool instead of PartiolOrd as return value of the comparison closure in {slice,Iteraotr}::is_sorted_by #118811

Merged
merged 1 commit into from
Jan 21, 2024

Conversation

EbbDrop
Copy link
Contributor

@EbbDrop EbbDrop commented Dec 10, 2023

Changes the function signature of the closure given to {slice,Iteraotr}::is_sorted_by to return a bool instead of a PartiolOrd as suggested by the libs-api team here: #53485 (comment).

This means these functions now return true if the closure returns true for all the pairs of values.

@rustbot
Copy link
Collaborator

rustbot commented Dec 10, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (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 (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 10, 2023
@rust-log-analyzer

This comment has been minimized.

@KisaragiEffective
Copy link
Contributor

I have following concern:

  • It is breaking change
  • It is harder to remember which direction is expected to inequality predicate.

@the8472
Copy link
Member

the8472 commented Dec 14, 2023

Is sorted is an unstable feature, so breaking changes are allowed.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 16, 2023
@WaffleLapkin
Copy link
Member

(this is marked as S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. , please use @rustbot review once this is ready for review) (learn more about rustbot commands)

@EbbDrop
Copy link
Contributor Author

EbbDrop commented Jan 12, 2024

@rustbot review

Ty for the pointer @WaffleLapkin

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 12, 2024
@Mark-Simulacrum
Copy link
Member

r=me with commits squashed. Thanks!

@EbbDrop
Copy link
Contributor Author

EbbDrop commented Jan 20, 2024

@bors r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Jan 20, 2024

@EbbDrop: 🔑 Insufficient privileges: Not in reviewers

@EbbDrop
Copy link
Contributor Author

EbbDrop commented Jan 20, 2024

@Mark-Simulacrum Looks like i am not allowed to use that bors command

@Mark-Simulacrum
Copy link
Member

@bors r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Jan 20, 2024

📌 Commit 606eeb8 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 20, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 21, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#116090 (Implement strict integer operations that panic on overflow)
 - rust-lang#118811 (Use `bool` instead of `PartiolOrd` as return value of the comparison closure in `{slice,Iteraotr}::is_sorted_by`)
 - rust-lang#119081 (Add Ipv6Addr::is_ipv4_mapped)
 - rust-lang#119461 (Use an interpreter in MIR jump threading)
 - rust-lang#119996 (Move OS String implementation into `sys`)
 - rust-lang#120015 (coverage: Format all coverage tests with `rustfmt`)
 - rust-lang#120027 (pattern_analysis: Remove `Ty: Copy` bound)
 - rust-lang#120084 (fix(rust-analyzer): use new pkgid spec to compare)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e8d1c2e into rust-lang:master Jan 21, 2024
11 checks passed
@rustbot rustbot added this to the 1.77.0 milestone Jan 21, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 21, 2024
Rollup merge of rust-lang#118811 - EbbDrop:is-sorted-by-bool, r=Mark-Simulacrum

Use `bool` instead of `PartiolOrd` as return value of the comparison closure in `{slice,Iteraotr}::is_sorted_by`

Changes the function signature of the closure given to `{slice,Iteraotr}::is_sorted_by` to return a `bool` instead of a `PartiolOrd` as suggested by the libs-api team here: rust-lang#53485 (comment).

This means these functions now return true if the closure returns true for all the pairs of values.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants