Skip to content

Conversation

Eh2406
Copy link
Contributor

@Eh2406 Eh2406 commented Sep 6, 2023

What does this PR try to resolve?

In #12629 (review) Ed pointed out how much cleaner the code can be using strip_prefix, so I found a bunch more places where we should be using it.

How should we test and review this PR?

Internal refactor and test still pass.

@rustbot
Copy link
Collaborator

rustbot commented Sep 6, 2023

r? @epage

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-cache-messages Area: caching of compiler messages A-cli Area: Command-line interface, option parsing, etc. A-dependency-resolution Area: dependency resolution and the resolver A-lockfile Area: Cargo.lock issues A-testing-cargo-itself Area: cargo's tests S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 6, 2023
Comment on lines 339 to +341
for (k, v) in metadata.iter().filter(|p| p.0.starts_with(prefix)) {
to_remove.push(k.to_string());
let k = &k[prefix.len()..];
let k = k.strip_prefix(prefix).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: you could turn the filter into a filter_map but then each iteration would have two different keys so unsure if its worth it

Copy link
Contributor

@epage epage left a comment

Choose a reason for hiding this comment

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

Looks good. Feel free to r= me

@rustbot rustbot added the A-cfg-expr Area: Platform cfg expressions label Sep 7, 2023
@Eh2406
Copy link
Contributor Author

Eh2406 commented Sep 7, 2023

I found a lot more.

@Eh2406
Copy link
Contributor Author

Eh2406 commented Sep 7, 2023

@bors r=epage

@bors
Copy link
Contributor

bors commented Sep 7, 2023

📌 Commit 44666f7 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 Sep 7, 2023
@bors
Copy link
Contributor

bors commented Sep 7, 2023

⌛ Testing commit 44666f7 with merge f399c21...

bors added a commit that referenced this pull request Sep 7, 2023
Ues strip_prefix for cleaner code

### What does this PR try to resolve?

In #12629 (review) Ed pointed out how much cleaner the code can be using `strip_prefix`, so I found a bunch more places where we should be using it.

### How should we test and review this PR?

Internal refactor and test still pass.
@bors
Copy link
Contributor

bors commented Sep 7, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 7, 2023
@Eh2406
Copy link
Contributor Author

Eh2406 commented Sep 8, 2023

I don't understand the error report, I don't think I made a breaking change.

@ehuss
Copy link
Contributor

ehuss commented Sep 8, 2023

BumpCheck compare against 9c4383fb55986096b414d98125421ab87b5fd642
error: Detected changes in these crates but no version bump found:
cargo-platform@0.1.4

Please bump at least one patch version in each corresponding Cargo.toml.

The check isn't for just semver-breaking changes, but for any change. If there aren't any semver-breaking changes, then you just need to bump the patch version.

Co-authored-by: Eric Huss <eric@huss.org>
@Eh2406
Copy link
Contributor Author

Eh2406 commented Sep 8, 2023

@bors r=epage

@bors
Copy link
Contributor

bors commented Sep 8, 2023

📌 Commit 2b28383 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 Sep 8, 2023
@bors
Copy link
Contributor

bors commented Sep 8, 2023

⌛ Testing commit 2b28383 with merge 8bd3231...

@bors
Copy link
Contributor

bors commented Sep 8, 2023

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

@bors bors merged commit 8bd3231 into rust-lang:master Sep 8, 2023
@Eh2406 Eh2406 deleted the strip_prefix branch September 8, 2023 21:12
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 9, 2023
Update cargo

14 commits in d14c85f4e6e7671673b1a1bc87231ff7164761e1..2fc85d15a542bfb610aff7682073412cf635352f
2023-09-05 22:28:10 +0000 to 2023-09-09 01:49:46 +0000
- feat: Stabilize lints (rust-lang/cargo#12648)
- Ues strip_prefix for cleaner code (rust-lang/cargo#12631)
- fix: don't print _TOKEN suggestion when not applicable (rust-lang/cargo#12644)
- Bump cargo-credential-1password to v0.4.0 (rust-lang/cargo#12641)
- refactor: put `Source` trait under `cargo::sources` (rust-lang/cargo#12527)
- Error out if `cargo clean --doc` is mixed with `-p`. (rust-lang/cargo#12637)
- Add wrappers around std::fs::metadata (rust-lang/cargo#12636)
- Add with_stdout_unordered. (rust-lang/cargo#12635)
- Fix example for creating a git project test. (rust-lang/cargo#12632)
- Read/write the encoded `cargo update --precise` in the same place (rust-lang/cargo#12629)
- docs(guide): Apply feedback on CI (rust-lang/cargo#12630)
- fix: improve warning for both token & credential-provider (rust-lang/cargo#12626)
- Skip clean up `profile.release.package."*"` (rust-lang/cargo#12624)
- Add MSRV validation GitHub Action for cargo-credential (rust-lang/cargo#12623)
@ehuss ehuss added this to the 1.74.0 milestone Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cache-messages Area: caching of compiler messages A-cfg-expr Area: Platform cfg expressions A-cli Area: Command-line interface, option parsing, etc. A-dependency-resolution Area: dependency resolution and the resolver A-lockfile Area: Cargo.lock issues A-testing-cargo-itself Area: cargo's tests 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