-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
fix indefinite article in cell.rs #79983
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @sfackler (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
@@ -1532,7 +1532,7 @@ impl<T: ?Sized + fmt::Display> fmt::Display for RefMut<'_, T> { | |||
/// `UnsafeCell<T>` is a type that wraps some `T` and indicates unsafe interior operations on the | |||
/// wrapped type. Types with an `UnsafeCell<T>` field are considered to have an 'unsafe interior'. | |||
/// The `UnsafeCell<T>` type is the only legal way to obtain aliasable data that is considered | |||
/// mutable. In general, transmuting an `&T` type into an `&mut T` is considered undefined behavior. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This depends how you pronounce &T
. I say "a reference type", but it sounds like the original author pronounced it "and-t".
Git says the author was @alexcrichton (e5da6a7) - Alex, what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That could be true...but couple of lines later you can see it being written the way i did it.
Line 1552 in 2b9ba46
/// - If you create a safe reference with lifetime `'a` (either a `&T` or `&mut T` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And then below again, it uses an
:
Line 1581 in 2b9ba46
/// accesses (_e.g._, through an `&mut UnsafeCell<_>`): neither the cell nor the wrapped value |
I think either they should all be consistent (and probably this would need a tracking issue because there's so many places it's inconsistent now), or it's not worth changing. I don't think it makes sense to just change this one article.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you create an issue, I can go for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, but we should decide whether to use 'a' or 'an' first. @steveklabnik do you have an opinion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I indeed pronounce this "and-t" and "and-mute-t", but I do not have an opinion on whether it's right or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it's tricky because it technically depends on how you pronounce it. I do what @alexcrichton does most of the time, though often "amp" and not "and," not that that changes the rules here.
https://github.com/rust-lang/rfcs/blob/master/text/1574-more-api-documentation-conventions.md is the canonical reference for how we word things. It does not directly address this issue, though it does contain the phrase "an &str
," probably because of "string slice" rather than &T
where T
= str
."
words are hard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I indeed pronounce this "and-t" and "and-mute-t", but I do not have an opinion on whether it's right or not.
I honestly couldn't imagine how you are pronouncing this and it being correct.
Sorry but can you write out the an exact phrase? The only thing I have in my head is an reference type
and an mutable reference type
which are obviously wrong.
r? @Dylan-DPC looks good to me and seems consistent with others |
Triage: What's next steps here? |
This needs
The hard part is 1, the others just take time. Personally I think this isn't worth spending a lot of time on, but I'm happy to review PRs if we can somehow decide which article to use. I do not have a strong preference on which article. |
@steveklabnik let me know your thoughts on this |
My thoughts are: language is not statically typed, either is fine. I don't have a preference. |
Going to merge this as is, we can make it consistent later. @bors r+ rollup |
📌 Commit 2b9ba46 has been approved by |
fix indefinite article in cell.rs
fix indefinite article in cell.rs
Rollup of 16 pull requests Successful merges: - rust-lang#79983 (fix indefinite article in cell.rs) - rust-lang#81831 (Don't display `mut` in arguments for functions documentation) - rust-lang#81947 (Relax ItemCtxt::to_ty lifetime) - rust-lang#81954 (RELEASES.md 1.50: Group platform support notes together) - rust-lang#81955 (bootstrap: Locate llvm-dwp based on llvm-config bindir) - rust-lang#81959 (Fix assosiated typo) - rust-lang#81964 (Fix documentation not showing on localStorage error) - rust-lang#81968 (bootstrap: fix wrong docs installation path) - rust-lang#81990 (Make suggestion of changing mutability of arguments broader) - rust-lang#81994 (Improve long explanation for E0542 and E0546) - rust-lang#81997 (dist: include src/build_helper as part of the crate graph for rustc-dev) - rust-lang#82003 (Stack probes: fix error message) - rust-lang#82004 (clean up clean::Static struct) - rust-lang#82011 (Fix private intra-doc warnings on associated items) - rust-lang#82013 (Tell user how to fix CI file being not up to date) - rust-lang#82017 (Fix typo in mod.rs) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
No description provided.