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

refactor: make resolve_with_previous clearer #13727

Merged
merged 4 commits into from
Apr 10, 2024
Merged

Conversation

weihanglo
Copy link
Member

What does this PR try to resolve?

Added more comments and extract patch registration to a function from resolve_with_previous, so we'll have both register_previous_lock and register_patch_entries looking symmetrical.

How should we test and review this PR?

There should have no behavior change.

The construction of pre_patch_keep closure is moved into cargo update module, since it is the only place using the closure, and I can't foresee anything will use it.

Additional information

@rustbot
Copy link
Collaborator

rustbot commented Apr 9, 2024

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-registries Area: registries Command-generate-lockfile S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 9, 2024
} else {
HashSet::new()
};

let keep = |p: &PackageId| pre_patch_keep(p) && !avoid_patch_ids.contains(p);
// Refine `keep` with patches that should avoid locking.
let keep = |p: &PackageId| keep(p) && !avoid_patch_ids.contains(p);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I'm normally a fan of shadowing, I feel like decorating a closure with another closure is likely going to make this easy to miss which could make reading the code confusing

@epage
Copy link
Contributor

epage commented Apr 10, 2024

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 10, 2024

📌 Commit 80d0d94 has been approved by epage

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 Apr 10, 2024
@bors
Copy link
Collaborator

bors commented Apr 10, 2024

⌛ Testing commit 80d0d94 with merge e366699...

@bors
Copy link
Collaborator

bors commented Apr 10, 2024

☀️ Test successful - checks-actions
Approved by: epage
Pushing e366699 to master...

1 similar comment
@bors
Copy link
Collaborator

bors commented Apr 10, 2024

☀️ Test successful - checks-actions
Approved by: epage
Pushing e366699 to master...

@bors bors merged commit e366699 into rust-lang:master Apr 10, 2024
21 checks passed
@bors
Copy link
Collaborator

bors commented Apr 10, 2024

👀 Test was successful, but fast-forwarding failed: 422 Changes must be made through a pull request.

@weihanglo weihanglo deleted the patch branch April 10, 2024 16:50
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 10, 2024
Update cargo

11 commits in 28e7b2bc0a812f90126be30f48a00a4ada990eaa..74fd5bc730b828dbc956335b229ac34ba47f7ef7
2024-04-05 19:31:01 +0000 to 2024-04-10 18:40:49 +0000
- chore: downgrade to openssl v1.1.1 (again) (rust-lang/cargo#13731)
- fix(cargo-fix): dont apply same suggestion twice (rust-lang/cargo#13728)
- refactor: make `resolve_with_previous` clearer (rust-lang/cargo#13727)
- fix(package): Normalize paths in `Cargo.toml` (rust-lang/cargo#13729)
- refactor: Track when MSRV is explicitly set, either way (rust-lang/cargo#13732)
- [fix]:Build script not rerun when target rustflags change (rust-lang/cargo#13560)
- feat(add): Stabilize MSRV-aware version req selection (rust-lang/cargo#13608)
- Fix github fast path redirect. (rust-lang/cargo#13718)
- Add release notes for 1.77.1 (rust-lang/cargo#13717)
- doc(semver): remove mention of deprecated tool rust-semverver (rust-lang/cargo#13715)
- chore: fix some typos (rust-lang/cargo#13714)

r? ghost
@rustbot rustbot added this to the 1.79.0 milestone Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-registries Area: registries Command-generate-lockfile S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants