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

Ban dashes in miropt test file names #103904

Merged
merged 1 commit into from
Nov 3, 2022
Merged

Conversation

JakobDegen
Copy link
Contributor

Dashes in miropt test file names are annoying - specifically, they cause the test files to not be sorted next to their outputs, which makes it very difficult to tell what goes with what. This PR changes all files to use underscores and adds a blessable tidy check that enforces this.

r? @jyn514

This PR is obviously going to bitrot very quickly. Because of this, I'm going to wait at least 48 hours and until review comes back to allow people from @rust-lang/wg-mir-opt or elsewhere to lodge complaints if there are any. If not, I'll rebase the branch on Friday afternoon PST and ask that it be r+'d with p=1 at that time.

@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 2, 2022
@jyn514 jyn514 added the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Nov 2, 2022
Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

The tidy code LGTM :) I'll let someone from WG-mir-opt say whether they want this change, but it makes sense to me.

src/tools/tidy/src/mir_opt_tests.rs Outdated Show resolved Hide resolved
@jyn514 jyn514 removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 2, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Nov 3, 2022

@bors r+ p=1 bitrotty

@bors
Copy link
Contributor

bors commented Nov 3, 2022

📌 Commit 0bd4f76 has been approved by oli-obk

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-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Nov 3, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Nov 3, 2022

This has annoyed me time and time again. Thank you for fixing it

@bors
Copy link
Contributor

bors commented Nov 3, 2022

⌛ Testing commit 0bd4f76 with merge 160b194...

@bors
Copy link
Contributor

bors commented Nov 3, 2022

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 160b194 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 3, 2022
@bors bors merged commit 160b194 into rust-lang:master Nov 3, 2022
@rustbot rustbot added this to the 1.67.0 milestone Nov 3, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (160b194): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.5% [3.5%, 3.5%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.3% [-2.9%, -1.7%] 2
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

@JakobDegen
Copy link
Contributor Author

Surprised that survived even 9 hours without conflicts 😂

@JakobDegen JakobDegen deleted the mir-opt-dash branch November 4, 2022 06:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants