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

Lowercase and remove periods in error messages for consistency #8655

Merged
merged 3 commits into from
Sep 8, 2020

Conversation

camelid
Copy link
Member

@camelid camelid commented Aug 27, 2020

No description provided.

@rust-highfive
Copy link

r? @ehuss

(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 Aug 27, 2020
@camelid
Copy link
Member Author

camelid commented Aug 27, 2020

Will this cause rustc tests to fail?

@camelid
Copy link
Member Author

camelid commented Aug 27, 2020

I ran:

$ fgrep 'Could not document' -r src/test

in rust-lang/rust and nothing came up, so I assume that there are no rustc tests that would be affected.

@camelid camelid closed this Aug 27, 2020
@camelid camelid deleted the patch-1 branch August 27, 2020 02:57
@camelid camelid restored the patch-1 branch August 27, 2020 02:57
@camelid
Copy link
Member Author

camelid commented Aug 27, 2020

It looks like there are a few more error messages that start with an uppercase letter and/or end with a period.

@camelid camelid reopened this Aug 27, 2020
@camelid
Copy link
Member Author

camelid commented Aug 27, 2020

E.g.:

$ grep '"Could not .*"' -r src
src/cargo/util/important_paths.rs:        anyhow::bail!("Could not find `{}` in `{}`", file, pwd.display())
src/cargo/util/process_builder.rs:                return Err(process_error("Could not set Ctrl-C handler.", None, None).into());
src/cargo/ops/cargo_read_manifest.rs:                "Could not find Cargo.toml in `{}`",

@camelid camelid changed the title Lowercase docs error message for consistency Lowercase and remove periods in error message for consistency Aug 27, 2020
@camelid camelid changed the title Lowercase and remove periods in error message for consistency Lowercase and remove periods in error messages for consistency Aug 27, 2020
@ehuss
Copy link
Contributor

ehuss commented Aug 31, 2020

Looks like there are some tests that need to be updated.

If you want to update those other error messages, that would be fine, up to you!

@camelid
Copy link
Member Author

camelid commented Sep 6, 2020

Is there any equivalent of ./x.py test --bless that I can use here?

@camelid
Copy link
Member Author

camelid commented Sep 6, 2020

What should I do about this message?

anyhow::bail!(
"manifest_path:{:?} is not an absolute path. Please provide an absolute path.",
manifest_path
)

@camelid
Copy link
Member Author

camelid commented Sep 7, 2020

It looks like there are a ton of error messages that need to be updated (around 40, by my estimate), so it might not be worth it for now ;)

I think I'll stick with only updating the overall "could not document" messages.

@ehuss
Copy link
Contributor

ehuss commented Sep 7, 2020

Is there any equivalent of ./x.py test --bless that I can use here?

Unfortunately there is not.

What should I do about this message?

The message is probably OK, I believe it is only for library users anyways.

I think I'll stick with only updating the overall "could not document" messages.

That's totally fine!

@camelid
Copy link
Member Author

camelid commented Sep 7, 2020

CI should hopefully pass now!

@camelid
Copy link
Member Author

camelid commented Sep 7, 2020

Okay, all tests are updated and CI is passing!

@ehuss
Copy link
Contributor

ehuss commented Sep 8, 2020

Thanks!

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 8, 2020

📌 Commit a9424d9 has been approved by ehuss

@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, 2020
@camelid
Copy link
Member Author

camelid commented Sep 8, 2020

Hmm, I wonder why bors isn't testing. There aren't any PRs ahead of this one in the queue:

image

@ehuss ehuss closed this Sep 8, 2020
@ehuss ehuss reopened this Sep 8, 2020
@ehuss
Copy link
Contributor

ehuss commented Sep 8, 2020

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 8, 2020

📌 Commit 82c834c has been approved by ehuss

@bors
Copy link
Collaborator

bors commented Sep 8, 2020

⌛ Testing commit 82c834c with merge 875e012...

@camelid
Copy link
Member Author

camelid commented Sep 8, 2020

Now it's working! Thanks :)

@ehuss
Copy link
Contributor

ehuss commented Sep 8, 2020

Yea, I think the bot was recently restarted, and it sometimes gets confused when that happens.

@camelid
Copy link
Member Author

camelid commented Sep 8, 2020

bors has been a bit sluggish recently on other things too.

@bors
Copy link
Collaborator

bors commented Sep 8, 2020

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing 875e012 to master...

@bors bors merged commit 875e012 into rust-lang:master Sep 8, 2020
@camelid camelid deleted the patch-1 branch September 8, 2020 20:47
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Sep 8, 2020
Update cargo

8 commits in 126907a7cfccbe93778530e6a6bbaa3adb6c515c..875e0123259b0b6299903fe4aea0a12ecde9324f
2020-08-31 20:42:11 +0000 to 2020-09-08 20:17:21 +0000
- Lowercase and remove periods in error messages for consistency (rust-lang/cargo#8655)
- Allow running build-man.sh from any directory (rust-lang/cargo#8682)
- docs: add details for cargo check pass where cargo build fail (rust-lang/cargo#8677)
- Fix nightly exported_priv_warning test. (rust-lang/cargo#8678)
- fix mdbook test with ```ignore/text/sh/console (rust-lang/cargo#8674)
- End CACHEDIR.TAG with newline (rust-lang/cargo#8672)
- Fixed the fossil repo initialization actually run commands (rust-lang/cargo#8671)
- Remove asciidoc attribute in cargo-metadata man page. (rust-lang/cargo#8670)
@ehuss ehuss added this to the 1.48.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

4 participants