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 find_map with any, first, and last variants #627

Merged
merged 1 commit into from
Feb 19, 2019

Conversation

seanchen1991
Copy link
Contributor

@seanchen1991 seanchen1991 commented Jan 20, 2019

PR opened in response to #607.

Copy link
Member

@cuviper cuviper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Can you please squash your commits? I don't mind multiple commits in a PR when they show progressive growth of a feature, but it looks like you're showing a few design iterations here, e.g. adding and then removing find_map.rs. Let's squash that to your final clean result.

Please run your additions through rustfmt too, as there are some differences.

Add non-working test for find_map_first

Got find_map_first test compiling

Get first test passing

Remove unused file

Need to write more comprehensive tests

Add tests for find_map_any

Add last newline back to find_first_last
@cuviper
Copy link
Member

cuviper commented Feb 19, 2019

FYI, GitHub doesn't always send notifications when you push new commits, so it helps to leave a comment. I also amended your commit with cargo fmt.

bors r+

bors bot added a commit that referenced this pull request Feb 19, 2019
627: Add `find_map` with `any`, `first`, and `last` variants r=cuviper a=seanchen1991

PR opened in response to #607. 

633: Update dev and demo dependencies r=cuviper a=cuviper

- `cgmath 0.17`
- `glium 0.23`
- `rand 0.6`

Co-authored-by: Sean <[email protected]>
Co-authored-by: Josh Stone <[email protected]>
@bors
Copy link
Contributor

bors bot commented Feb 19, 2019

@bors bors bot merged commit 8fbb588 into rayon-rs:master Feb 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants