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

Ues strip_prefix for cleaner code #12631

Merged
merged 3 commits into from
Sep 8, 2023
Merged

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

src/cargo/util/rustc.rs Outdated Show resolved Hide resolved
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.

tests/testsuite/ssh.rs Outdated Show resolved Hide resolved
@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
21 checks passed
@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