Skip to content
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

Add Option::filter() according to RFC 2124 #44996

Closed

Conversation

LukasKalbertodt
Copy link
Member

@LukasKalbertodt LukasKalbertodt commented Oct 3, 2017

This is the implementation of the not yet accepted RFC "Add Option::filter to the standard library". However, the feedback on the RFC is very good so far and I think it's rather unlikely the RFC will be rejected.

TODO:

Questions for code reviewers:

  • Is the documentation sufficiently long?
  • Is the documentation easy enough to understand?
  • Is the position of the new method (after and_then()) a good one?

@aidanhs aidanhs added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 5, 2017
@aidanhs
Copy link
Member

aidanhs commented Oct 5, 2017

Thanks for the PR @LukasKalbertodt! I've set @sfackler as the reviewer for the PR but have set it as "with author" for now until the RFC gets merged - assuming it does, we can than get this reviewed and merged.

@aidanhs
Copy link
Member

aidanhs commented Oct 12, 2017

In fact, I think I'll close this PR for now as I don't see the RFC being merged in the next week - once it is, please ping me and I'll reopen and we can get your PR reviewed :)

(this is just so PR triagers aren't constantly visiting this PR and checking to see where the RFC is up to)

@aidanhs aidanhs closed this Oct 12, 2017
@LukasKalbertodt
Copy link
Member Author

@aidanhs Sure! Sorry for opening this PR too soon, I expected the RFC to be merged earlier :)

@LukasKalbertodt
Copy link
Member Author

@aidanhs The RFC has been merged and I inserted the correct tracking issue id into the code. This PR can be opened again :)

@LukasKalbertodt LukasKalbertodt changed the title WIP: Add Option::filter() according to RFC 2124 Add Option::filter() according to RFC 2124 Nov 8, 2017
@kennytm
Copy link
Member

kennytm commented Nov 8, 2017

screenshot_2017-11-08 17 27 50_xmed2y

😕 It may be easier to just submit a new PR (isaacs/github#361).

@LukasKalbertodt
Copy link
Member Author

Oopsie. Okay dokey!

kennytm added a commit to kennytm/rust that referenced this pull request Nov 10, 2017
…r=dtolnay

Add `Option::filter()` according to RFC 2124

(*old PR: rust-lang#44996)

This is the implementation of [RFC "Add `Option::filter` to the standard library"](rust-lang/rfcs#2124). Tracking issue: rust-lang#45860

**Questions for code reviewers:**

- Is the documentation sufficiently long?
- Is the documentation easy enough to understand?
- Is the position of the new method (after `and_then()`) a good one?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants