generics/phantom/testcase_units: alleviate the syntactical heaviness of the types and bounds #583
Conversation
…of the types and bounds
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @steveklabnik (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. 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. |
There was a problem hiding this comment.
This isn't quite right. Add<RHS = Self> seems to force RHS to default to Self. So:
impl Add<f64> for T {} // T + f64 (override default)
impl Add for T // T + T (RHS is Self because of the default)There is no defaults section though probably because I couldn't figure them out when I was working on the associated types sections.
There was a problem hiding this comment.
Note: this isn't your mistake. It just wasn't written correctly before.
It's mainly an issue because it uses <X = Y> but the actual example doesn't require that part.
There was a problem hiding this comment.
Apologies, I missed that comment yesterday... Well I didn't know about that defaulting mechanism indeed, it's nice.
I updated the comment to mention that. I would have removed that line but since it's not explained anywhere else, it does get confusing. Let me know if there's anything else.
|
This seems good other than that assuming @steveklabnik doesn't have issues with it. |
|
I removed one of the blocks as you suggested. I didn't do it at first because I wasn't sure how much you wanted me to depart from the initial example. But you're right, keeping the two was probably not necessary. |
|
Awesome. If the other nit is fixed this seems good to me. |
|
Nice! Looks good to me. Now just wait for official review from @steveklabnik. |
There was a problem hiding this comment.
since you've added below, the period at the end here should be removed
|
@steveklabnik you probably want to skim #582 where other options are discussed for changing this as well. Note: I'm not currently inclined to make any of the changes suggested but someone else might take it and run with it. |
|
No worries... You're welcome, and ty the website was I've tried to follow your instructions, although there's still one or two details I'm not sure about, mostly regarding periods. |
There was a problem hiding this comment.
I moved the period to the end of the sentence. I'm not sure if you wanted it removed completely, but I thought that would be inconsistent with line 13.
There was a problem hiding this comment.
Overall, we may have inconsistencies. Rust by Example was donated to the Rust organization, and I haven't had the chance to go over the entire thing yet. So, when adding new stuff, I tend to apply our already-existing conventions, even though the project as a whole may not have totally adopted them yet. Make sense?
|
I'm glad the sites helpful. The site doesn't get many critiques anymore. They're all directed at the book |
|
Ah! I didn't see that this was fixed. Thanks so much @letheed ! |
generics/phantom/testcase_units: alleviate the syntactical heaviness of the types and bounds
4 participants
I tried to retain the information on the bounds in a few comments because it does matter.
Close #582