-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Documentation change: Now showing the difference between map
and and_then
with an example.
#30971
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (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. |
Thanks! Could you also wrap the lines to 80 chars (appears to be the surrounding style). For the actual content, though: |
I just felt the manually inserted 80 char wide line breaks are an anachronism in this day and age. Almost all editors can do word wrapping now. The reader should be given a choice of where they want to see the line breaks instead of making this decision for them. IMO line breaks should only be used to delineate paragraphs. Also I think unnecessary hard line breaks will make whitespace significant during file merge/comparison and make it unnecessarily hard to contribute. For example you might overflow the 80 char limit when adding a word to a line and end up with a cascading change which affects multiple lines thus making it difficult to see what was actually changed. There may be other complications arising out of this decision. I vote for removing hard line breaks from the document altogether. But I do not want to upset the apple cart on the other hand so please let me know if you would like to go with ...
|
Er sorry but this is just the style we use, changing it isn't really within the scope of this PR. |
@alexcrichton, changed to 80 char column now. |
@bors: r+ rollup Thank you! |
📌 Commit 5f20143 has been approved by |
⌛ Testing commit 5f20143 with merge 1998e3a... |
💔 Test failed - auto-mac-64-nopt-t |
implementation is even simpler than `map`: | ||
analysis, but its type doesn't quite fit... | ||
|
||
```rust |
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 needs to be marked ignore
as well, so that it doesn't get tested.
Updated documentation to clarify the difference between `and_then` and `map`. This also explains why we need `and_then` in addition to `map`. Please look at the diff for more information. r? @alexcrichton
@bors: r- |
@steveklabnik please review and commit. |
@bors: r+ rollup thank you! |
📌 Commit 0922d7e has been approved by |
Updated documentation to clarify the difference between `and_then` and `map`. This also explains why we need `and_then` in addition to `map`. Please look at the diff for more information. r? @alexcrichton
Updated documentation to clarify the difference between `and_then` and `map`. This also explains why we need `and_then` in addition to `map`. Please look at the diff for more information. r? @alexcrichton
Updated documentation to clarify the difference between
and_then
andmap
. This also explains why we needand_then
in addition tomap
. Please look at the diff for more information.r? @alexcrichton