Cosmetic improvements to doc comments #58341
Conversation
rust-highfive
added
the
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
| @@ -66,7 +66,7 @@ impl Step for Rustc { | |||
| }); | |||
| } | |||
|
|
|||
| /// Build the compiler. | |||
| /// Builds the compiler. | |||
There was a problem hiding this comment.
Why is this an improvement rather than a random stylistic change?
What will prevent people from using their preferred form again?
=> controversial
There was a problem hiding this comment.
Not controversial at all. This is the standard/conventional style, to use the present tense, and improves consistency too. What prevents this happening again is people like you and me making sure this is used when reviewing PRs.
There was a problem hiding this comment.
"Build the compiler" is also present tense, so that's not even the point here. The difference is about using 2nd or 3rd person singular.
But indeed the majority of doc comments seem to be written in 3rd person.
There was a problem hiding this comment.
@RalfJung
[ot] I never realized imperative is actually a usual present tense with implicit "you" in English, it's an entirely separate form without a tense in Russian.
There was a problem hiding this comment.
Yeah same in German... English just has the most boring verbs.^^
I guess it's actually meant to be imperative here, not 2nd person singular. Whatever.
There was a problem hiding this comment.
@RalfJung @petrochenkov Well yes, but it's the imperative mood... the issue is so many verb forms are degenerate in English. In any case, the convention seems to be 3rd person singular present indicative. ;-) I.e., "[This function] builds the compiler." This makes sense to me.
This comment has been minimized.
This comment has been minimized.
alexreg
force-pushed
the
cosmetic-2-doc-comments
branch
from
a431544
to
4d7303a
Compare
|
☔ The latest upstream changes (presumably #58129) made this pull request unmergeable. Please resolve the merge conflicts. |
alexreg
force-pushed
the
cosmetic-2-doc-comments
branch
from
4d7303a
to
f438305
Compare
|
Okay, looks like tests are passing and everything is good to go, in theory... |
alexreg
force-pushed
the
cosmetic-2-doc-comments
branch
from
f438305
to
f943296
Compare
|
@bors: r+ p=1 Normally, I would not p=1 this, but given that it's very likely to bit-rot and we're splitting up a massive PR, let's do it. I was gonna do a rollup but there are only two other rollup PRs. It's monday morning, this is fine, imho. If someone strongly disagrees feel free to do something else. |
|
📌 Commit f943296 has been approved by |
bors
added
S-waiting-on-bors
|
⌛ Testing commit f943296 with merge a0e77211932fce19f61097d2344dd53934f56ac6... |
|
💔 Test failed - status-appveyor |
bors
added
S-waiting-on-review
|
Thanks @steveklabnik. Those were my thoughts too, essentially. It doesn't have the "importance" of a higher-priority PR, but due to the tendency to bitrot, etc. Looks like like an ephemeral failure with bors though, if I'm not mistaken? |
|
Tracking issue for this failure: #58025 It should work after |
bors
added
S-waiting-on-bors
This comment has been minimized.
This comment has been minimized.
bors
added
S-waiting-on-review
|
@bors retry |
bors
added
S-waiting-on-bors
Cosmetic improvements to doc comments This has been factored out from #58036 to only include changes to documentation comments (throughout the rustc codebase). r? @steveklabnik Once you're happy with this, maybe we could get it through with r=1, so it doesn't constantly get invalidated? (I'm not sure this will be an issue, but just in case...) Anyway, thanks for your advice so far!
|
☀️ Test successful - checks-travis, status-appveyor |
This has been factored out from #58036 to only include changes to documentation comments (throughout the rustc codebase).
r? @steveklabnik
Once you're happy with this, maybe we could get it through with r=1, so it doesn't constantly get invalidated? (I'm not sure this will be an issue, but just in case...) Anyway, thanks for your advice so far!