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.

Sign up for GitHub

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

Jump to bottom

test(z-flags): Verify -Z flags list is sorted #12224

Merged
merged 1 commit into from
Jun 11, 2023
Merged

test(z-flags): Verify -Z flags list is sorted #12224

merged 1 commit into from
Jun 11, 2023

Conversation

Muscraft
Copy link
Member

@Muscraft Muscraft commented Jun 2, 2023

#12182 made it so that unstable_cli_options! was sorted, but had no way to make sure it would stay sorted in the future.

In this thread, I proposed that a test should be added that would fail if the list is not sorted, this PR adds that test.

If the list is not sorted the test output looks like this:
Screenshot from 2023-06-02 10-17-08

@rustbot
Copy link
Collaborator

rustbot commented Jun 2, 2023

r? @weihanglo

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

@rustbot rustbot added A-unstable Area: nightly unstable support S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 2, 2023
weihanglo
weihanglo approved these changes Jun 2, 2023
View reviewed changes
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

This looks good to me. Feel free to r=weihanglo when you manage to fix the error.

src/cargo/core/features.rs Show resolved Hide resolved
@ehuss
Copy link
Contributor

ehuss commented Jun 2, 2023

Just a drive-by comment, I haven't really been following the changes here. Something to consider would be something like this tidy test which the compiler uses to keep lists of things sorted by using special comments to mark what should be sorted. I'm not sure if that would help in this case, but something to consider.

@epage epage mentioned this pull request Jun 9, 2023
Merged
bors added a commit that referenced this pull request Jun 9, 2023
@bors
Auto merge of #12246 - epage:lint, r=weihanglo
7a0b24b 
chore: Migrate print-ban from test to clippy

This should be more resilient to false positives like in #12245 where a string contains `println`.

See also #12224
@weihanglo weihanglo added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 9, 2023
@weihanglo
Copy link
Member

@bors try

@bors
Copy link
Contributor

bors commented Jun 11, 2023

⌛ Trying commit 8d143cc with merge e87d61f...

bors added a commit that referenced this pull request Jun 11, 2023
@bors
Auto merge of #12224 - Muscraft:test-z-flags-sorted, r=<try>
e87d61f 
test(z-flags): Verify `-Z` flags list is sorted

#12182 made it so that `unstable_cli_options!` was sorted, but had no way to make sure it would stay sorted in the future.

In [this thread](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Sorting.20features!.20and.20ensuring.20it.20stays.20sorted), I proposed that a test should be added that would fail if the list is not sorted, this PR adds that test.

If the list is not sorted the test output looks like this:
![Screenshot from 2023-06-02 10-17-08](https://github.com/rust-lang/cargo/assets/23045215/f22cf5f3-4411-44ec-ac5a-991165d5e0a1)
@weihanglo
Copy link
Member

Something to consider would be something like this tidy test which the compiler uses to keep lists of things sorted by using special comments to mark what should be sorted.

This is definitely a good long-term solution. We do have some other places that are better to keep sorted, such as

@weihanglo
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jun 11, 2023

📌 Commit 8d143cc has been approved by weihanglo

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-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. labels Jun 11, 2023
@bors
Copy link
Contributor

bors commented Jun 11, 2023

⌛ Testing commit 8d143cc with merge 18a28a1...

@bors
Copy link
Contributor

bors commented Jun 11, 2023

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 18a28a1 to master...

@bors bors merged commit 18a28a1 into rust-lang:master Jun 11, 2023
@Muscraft Muscraft deleted the test-z-flags-sorted branch June 13, 2023 04:37
@weihanglo weihanglo mentioned this pull request Jun 14, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 15, 2023
@bors
Auto merge of rust-lang#112601 - weihanglo:update-cargo, r=weihanglo
5a65be8 
Update cargo

11 commits in 49b6d9e179a91cf7645142541c9563443f64bf2b..0c14026aa84ee2ec4c67460c0a18abc8519ca6b2
2023-06-09 17:21:19 +0000 to 2023-06-14 18:43:05 +0000
- fix(embedded): Don't append hash to bin names (rust-lang/cargo#12269)
- Fix version requirement example in Dependency Resolution, SemVer compatibility section (rust-lang/cargo#12267)
- Update triagebot links. (rust-lang/cargo#12265)
- Show a better error when container tests fail. (rust-lang/cargo#12264)
- chore: update dependencies (rust-lang/cargo#12261)
- refactor(embedded) (rust-lang/cargo#12262)
- docs: clarify the use of `default` branch instead of `main` by default (rust-lang/cargo#12251)
- docs: update changelog for 1.71 backport and 1.72 (rust-lang/cargo#12256)
- feat: Initial support for single-file packages (rust-lang/cargo#12245)
- test(z-flags): Verify `-Z` flags list is sorted (rust-lang/cargo#12224)
- refactor: registry data kinds cleanup (rust-lang/cargo#12248)

---

This commit also update LICENSE exceptions, as Cargo introduced a newer version of `dunce` and `blake3` as dependencies.

r? `@ghost`
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Jun 16, 2023
@bors
Auto merge of #112601 - weihanglo:update-cargo, r=weihanglo
4f1474a 
Update cargo

11 commits in 49b6d9e179a91cf7645142541c9563443f64bf2b..0c14026aa84ee2ec4c67460c0a18abc8519ca6b2
2023-06-09 17:21:19 +0000 to 2023-06-14 18:43:05 +0000
- fix(embedded): Don't append hash to bin names (rust-lang/cargo#12269)
- Fix version requirement example in Dependency Resolution, SemVer compatibility section (rust-lang/cargo#12267)
- Update triagebot links. (rust-lang/cargo#12265)
- Show a better error when container tests fail. (rust-lang/cargo#12264)
- chore: update dependencies (rust-lang/cargo#12261)
- refactor(embedded) (rust-lang/cargo#12262)
- docs: clarify the use of `default` branch instead of `main` by default (rust-lang/cargo#12251)
- docs: update changelog for 1.71 backport and 1.72 (rust-lang/cargo#12256)
- feat: Initial support for single-file packages (rust-lang/cargo#12245)
- test(z-flags): Verify `-Z` flags list is sorted (rust-lang/cargo#12224)
- refactor: registry data kinds cleanup (rust-lang/cargo#12248)

---

This commit also update LICENSE exceptions, as Cargo introduced a newer version of `dunce` and `blake3` as dependencies.

r? `@ghost`
@ehuss ehuss added this to the 1.72.0 milestone Jun 22, 2023
epage added a commit to epage/cargo that referenced this pull request Dec 11, 2024
@epage
fix(script): Don't override the release profile
ab7469e 
This has been around since rust-lang#12224 and isn't in the RFC, so its being
removed.

This is part of rust-lang#12207.
@epage epage mentioned this pull request Dec 11, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Dec 12, 2024
@weihanglo
fix(script): Don't override the release profile (#14925)
3cd245c 
This has been around since #12224 and isn't in the RFC, so its being
removed.

This is part of #12207.

<!--
Thanks for submitting a pull request 🎉! Here are some tips for you:

* If this is your first contribution, read "Cargo Contribution Guide"
first:
  https://doc.crates.io/contrib/
* Run `cargo fmt --all` to format your code changes.
* Small commits and pull requests are always preferable and easy to
review.
* If your idea is large and needs feedback from the community, read how:
  https://doc.crates.io/contrib/process/#working-on-large-features
* Cargo takes care of compatibility. Read our design principles:
  https://doc.crates.io/contrib/design.html
* When changing help text of cargo commands, follow the steps to
generate docs:

https://github.com/rust-lang/cargo/tree/master/src/doc#building-the-man-pages
* If your PR is not finished, set it as "draft" PR or add "WIP" in its
title.
* It's ok to use the CI resources to test your PR, but please don't
abuse them.

### What does this PR try to resolve?

Explain the motivation behind this change.
A clear overview along with an in-depth explanation are helpful.

You can use `Fixes #<issue number>` to associate this PR to an existing
issue.

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

Demonstrate how you test this change and guide reviewers through your
PR.
With a smooth review process, a pull request usually gets reviewed
quicker.

If you don't know how to write and run your tests, please read the
guide:
https://doc.crates.io/contrib/tests

### Additional information

Other information you want to mention in this PR, such as prior arts,
future extensions, an unresolved problem, or a TODO list.
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weihanglo weihanglo weihanglo approved these changes

@epage epage epage left review comments

Assignees

@weihanglo weihanglo

Labels
A-unstable Area: nightly unstable support S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.72.0
Development

Successfully merging this pull request may close these issues.
6 participants
@Muscraft @rustbot @ehuss @weihanglo @bors @epage