Skip to content

Rewrite update-all-references bash scripts in Rust #6413

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

Merged
merged 3 commits into from
Dec 12, 2020

Conversation

phansch
Copy link
Member

@phansch phansch commented Dec 2, 2020

This replaces the update-all-references scripts with a single

cargo dev bless

command. It should behave mostly the same as the bash scripts. The major difference is, that it can be called from the project root and will always update the files in all of the test suites.

cc #5394

changelog: none

@rust-highfive
Copy link

r? @flip1995

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Dec 2, 2020
@phansch
Copy link
Member Author

phansch commented Dec 2, 2020

#5394 also mentions that bless should run the tests beforehand, but I'm not quite sure how to do that?

Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

This won't work in rustc I guess. But we can make this work in a later PR somehow.

}
}

fn update_test_file(test_file_path: PathBuf) {
Copy link
Member

Choose a reason for hiding this comment

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

NIT: Maybe a better naming would be

Suggested change
fn update_test_file(test_file_path: PathBuf) {
fn update_test_file(reference_file_path: PathBuf) {

and the same for other *_test_file* names.

And then for build_output, test_output instead?

With this the if clause below would become if test_output != reference_file, which is more intuitive to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree that's much better, thanks!

Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

r=me with CI passing

@phansch phansch added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Dec 12, 2020
This replaces the `update-all-references` scripts with a single

    cargo dev bless

command.

cc rust-lang#5394
@phansch
Copy link
Member Author

phansch commented Dec 12, 2020

@bors r=flip1995

@bors
Copy link
Contributor

bors commented Dec 12, 2020

📌 Commit b8501e1 has been approved by flip1995

@bors
Copy link
Contributor

bors commented Dec 12, 2020

⌛ Testing commit b8501e1 with merge 18c5ea4...

@bors
Copy link
Contributor

bors commented Dec 12, 2020

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 18c5ea4 to master...

@bors bors merged commit 18c5ea4 into rust-lang:master Dec 12, 2020
@phansch phansch deleted the bless branch December 12, 2020 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants