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

Update tidy check for error codes testing #66574

Merged
merged 1 commit into from
Nov 23, 2019
Merged

Update tidy check for error codes testing #66574

merged 1 commit into from
Nov 23, 2019

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Nov 20, 2019 •
edited by Mark-Simulacrum
Loading

Now that all the error codes have been grouped into one file, we moved the long error explanations into markdown files, which means that we need to read those markdown file to also count their code blocks as well.

So 2 "big" things happened here:

  • No more need to keep the current error code when reading the error_codes.rs file since it's been put into the equivalent markdown file.
  • Need to instead read the markdown file (but it's simpler since I can just look for code blocks directly).

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 20, 2019
@GuillaumeGomez
Copy link
Member Author

Updated.

@Mark-Simulacrum
Copy link
Member

Could you also say a bit about what this is doing? i.e., what is the goal of this PR, what are we trying to change, and why? I don't have the time to dig into that myself so it'd make an r+ easier on my end.

@GuillaumeGomez
Copy link
Member Author

Sure. Now that all the error codes have been grouped into one file, we moved the long error explanations into markdown files, which means that we need to read those markdown file to also count their code blocks as well.

So 2 "big" things happened here:

  • No more need to keep the current error code when reading the error_codes.rs file since it's been put into the equivalent markdown file.
  • Need to instead read the markdown file (but it's simpler since I can just look for code blocks directly).

Mark-Simulacrum
Mark-Simulacrum reviewed Nov 20, 2019
View reviewed changes
src/tools/tidy/src/error_codes_check.rs Outdated
let md = some_or_continue!(s.splitn(2, "include_str!(\"").skip(1).next());
let md_file_name = some_or_continue!(md.splitn(2, "\")").next());
let path = some_or_continue!(path.parent()).join(md_file_name);
match File::open(&path) {
Copy link
Member

Choose a reason for hiding this comment

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

Could we use fs::read_to_string here? Seems like there's no reason why not?

@Mark-Simulacrum
Copy link
Member

For the future it'd be great to include that information in the commit; I've copied it into the PR description.

r=me with last nit fixed

@GuillaumeGomez
Copy link
Member Author

Indeed, sorry...

@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Nov 21, 2019

📌 Commit df9fed1 has been approved by Mark-Simulacrum

@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 Nov 21, 2019
Centril added a commit to Centril/rust that referenced this pull request Nov 23, 2019
@Centril
Rollup merge of rust-lang#66574 - GuillaumeGomez:update-tidy-err-code…
8256e73 
…-check, r=Mark-Simulacrum

Update tidy check for error codes testing

Now that all the error codes have been grouped into one file, we moved the long error explanations into markdown files, which means that we need to read those markdown file to also count their code blocks as well.

So 2 "big" things happened here:
* No more need to keep the current error code when reading the `error_codes.rs` file since it's been put into the equivalent markdown file.
* Need to instead read the markdown file (but it's simpler since I can just look for code blocks directly).
@Centril Centril mentioned this pull request Nov 23, 2019
Merged
bors added a commit that referenced this pull request Nov 23, 2019
@bors
Auto merge of #66656 - Centril:rollup-fivygwz, r=Centril
d902539 
Rollup of 4 pull requests

Successful merges:

 - #65961 (add fn type_name_of_val)
 - #66574 (Update tidy check for error codes testing)
 - #66576 (made gdb pretty-printing more robust when printing uninitialized vec)
 - #66583 (Clarify Step Documentation)

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented Nov 23, 2019

⌛ Testing commit df9fed1 with merge d902539...

@bors bors merged commit df9fed1 into rust-lang:master Nov 23, 2019
@GuillaumeGomez GuillaumeGomez deleted the update-tidy-err-code-check branch November 23, 2019 11:00
@petrochenkov petrochenkov mentioned this pull request Nov 24, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mark-Simulacrum Mark-Simulacrum Mark-Simulacrum left review comments

Assignees

@Mark-Simulacrum Mark-Simulacrum

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@GuillaumeGomez @Mark-Simulacrum @bors @rust-highfive