Uh oh!
There was an error while loading. Please reload this page.
Make lifetime elision help more useful on type signatures. - #20263
Conversation
rust-highfive
commented
Dec 27, 2014
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nick29581 (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 CONTRIBUTING.md for more information. |
nrc
commented
Dec 27, 2014
Looks good! For the output I think there should only be backticks around the name of the argument if it is the actual name. If it is "argument #1" or whatever, then there shouldn't be backticks. Also, I would just use "argument 1" instead of "argument #1" because the user might wonder if the # has any special meaning (and indeed in future Rust it might have some special meaning), otherwise it is a good choice of output. |
There was a problem hiding this comment.
nit: please add spaces around the -> and after the first :
crhino
commented
Dec 28, 2014
That all makes sense to me, addressed in the commit above. |
There was a problem hiding this comment.
It would be more idiomatic Rust to re-assign to name and make it immutable (i.e., remove mut and add let name = ...
crhino
commented
Dec 28, 2014
Done. Should I rebase and squash? |
There was a problem hiding this comment.
You don't actually need to change the name to help_name, just keeping name is fine, but either way works for me
nrc
commented
Dec 29, 2014
@crhino yes please, r=me when you do |
0eb6f50 to
ba17759Comparecrhino
commented
Dec 29, 2014
It felt a little weird to keep the name as r=nick29581 |
Fixesrust-lang#19707. In terms of output, it currently uses the form `argument #1`, `argument #2`, etc. If anyone has any better suggestions I would be glad to consider them.
Fixes#19707.
In terms of output, it currently uses the form
argument #1,argument #2, etc. If anyone has any better suggestions I would be glad to consider them.