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.

Sign up for GitHub

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

Jump to bottom

Add lints for find_map #4039

Merged
merged 2 commits into from
Apr 30, 2019
Merged

Add lints for find_map #4039

merged 2 commits into from
Apr 30, 2019

Conversation

andrehjr
Copy link
Contributor

@andrehjr andrehjr commented Apr 27, 2019 •
edited
Loading

changelog: adds lints for find_map and filter_map_next. Closes issue #3474

Hope I got everything correctly this time! Let me know if I missed something.

@andrehjr andrehjr changed the title Add lints for find_map WIP: Add lints for find_map Apr 27, 2019
@andrehjr andrehjr marked this pull request as ready for review April 27, 2019 22:45
@andrehjr andrehjr force-pushed the add-find-map-lints branch from 2491c8d to f3f168e Compare April 28, 2019 18:19
@flip1995
Copy link
Member

LGTM. Do you want to add anything else or can the "WIP" in the title be removed?

@flip1995 flip1995 added the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Apr 30, 2019
@andrehjr andrehjr changed the title WIP: Add lints for find_map Add lints for find_map Apr 30, 2019
@andrehjr andrehjr force-pushed the add-find-map-lints branch from ccdf8f0 to 2683884 Compare April 30, 2019 16:12
@andrehjr
Copy link
Contributor Author

I removed the WIP, after the travis build finishes with success. Unless there are more comments it should be good to merge! Thanks! :)

@flip1995
Copy link
Member

I guess it's because the note gets to bloaty when the snippet is multiline. If this is done in other lints in this file, I agree that we should also do it here. But you can keep the test.

Thanks for trying this out!

@andrehjr
Copy link
Contributor Author

Awesome! just pushed the change bringing back the checks for multiliners

@flip1995
Copy link
Member

Thanks! If you squash these last few back and forth commits we can merge this :)

@andrehjr andrehjr force-pushed the add-find-map-lints branch from ca903ac to e428862 Compare April 30, 2019 19:46
@andrehjr
Copy link
Contributor Author

Done @flip1995 rebased and squashed a few things 🚀

@flip1995
Copy link
Member

Perfect

@bors r+

@bors
Copy link
Contributor

bors commented Apr 30, 2019

📌 Commit e428862 has been approved by flip1995

bors added a commit that referenced this pull request Apr 30, 2019
@bors
Auto merge of #4039 - andrehjr:add-find-map-lints, r=flip1995
e56e325 
Add lints for find_map

changelog: adds lints for find_map and filter_map_next. Closes issue #3474

Hope I got everything correctly this time! Let me know if I missed something.
@bors
Copy link
Contributor

bors commented Apr 30, 2019

⌛ Testing commit e428862 with merge e56e325...

@bors
Copy link
Contributor

bors commented Apr 30, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: flip1995
Pushing e56e325 to master...

@bors bors merged commit e428862 into rust-lang:master Apr 30, 2019
@bors bors mentioned this pull request Apr 30, 2019
Merged
@andrehjr andrehjr deleted the add-find-map-lints branch May 1, 2019 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flip1995 flip1995 flip1995 approved these changes

Assignees
No one assigned
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andrehjr @flip1995 @bors