-
Notifications
You must be signed in to change notification settings - Fork 13.3k
s/deriving/derives in Comments/Docs #20998
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
Conversation
r? @sfackler (rust_highfive has picked a reviewer for you, use r? to override) |
Seems like this could fix #20984. |
@@ -2427,13 +2427,13 @@ There are three different types of inline attributes: | |||
|
|||
### Deriving |
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 should get changed too.
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 almost changed that, but felt I should leave it up for comment. "derives" ?
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've tended to just make it
## `derive`
or similar, when a heading is talking about a specific feature. The section of if
does this, for example
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.
Maybe
Deriving traits with `derive`
?
Two small nits, and can you add "Fixes #20984" to the commit message? r=me after, thank you! |
@steveklabnik will do |
r=steveklabnik |
Thanks! (only people with commit can do the r= thing, and it needs to go on the commit, not on the PR) |
This failed to merge in the rollup :( |
@steveklabnik These all look like doc changes to me. I'll see if I can figure out what's going on. Sorry, I didn't realize the examples were also tested or I would have make checked this. |
It's not that the tests failed, it's that it failed to merge. So after the current rollup lands, you're going to need to rebase this. |
@steveklabnik got it. Thanks, will do. |
There are a large number of places that incorrectly refer to deriving in comments, instead of derives. Fixes rust-lang#20984
re-r? @steveklabnik |
There are a large number of places that incorrectly refer to deriving in comments, instead of derives. If someone could look at src/etc/generate-deriving-span-tests.py, I'm not sure how those tests were passing before/if they were.
@@ -132,7 +132,7 @@ pub fn cs_cmp(cx: &mut ExtCtxt, span: Span, | |||
cx.expr_path(equals_path.clone()), | |||
box |cx, span, (self_args, tag_tuple), _non_self_args| { | |||
if self_args.len() != 2 { | |||
cx.span_bug(span, "not exactly 2 arguments in `deriving(Ord)`") | |||
cx.span_bug(span, "not exactly 2 arguments in `derives(Ord)`") |
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.
Hm, this seems wrong :o should be derive
?
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 think you're write. I'll add another patch.
There are a large number of places that incorrectly refer
to deriving in comments, instead of derives.
If someone could look at src/etc/generate-deriving-span-tests.py,
I'm not sure how those tests were passing before/if they were.