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

manual_filter_map and manual_find_map #6591

Merged
merged 6 commits into from
Jan 22, 2021
Merged

manual_filter_map and manual_find_map #6591

merged 6 commits into from
Jan 22, 2021

Conversation

camsteffen
Copy link
Contributor

@camsteffen camsteffen commented Jan 15, 2021 •
edited
Loading

changelog: Add manual_filter_map and replace find_map with manual_find_map

Replaces #6453

Fixes #3188
Fixes #4193

Depends on #6567 (to fix an internal lint false positive)

This replaces filter_map and find_map with manual_filter_map and manual_find_map respectively. However, filter_map is left in place since it is used for a variety of other cases. See discussion in #6453.

@rust-highfive
Copy link

r? @llogiq

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 15, 2021
@camsteffen camsteffen mentioned this pull request Jan 15, 2021
Closed
@llogiq
Copy link
Contributor

llogiq commented Jan 15, 2021

Our internal path check isn't having the OPTION_IS_SOME and RESULT_IS_OK paths. Shouldn't we have symbols for those anyway?

@camsteffen
Copy link
Contributor Author

That will be fixed in #6567.

Hmm I suppose it is better to use the Option diagnostic item + method name. I'll do that.

@camsteffen camsteffen force-pushed the manual-filter-map branch 4 times, most recently from a5fbcc9 to 184694f Compare January 18, 2021 17:37
@llogiq
Copy link
Contributor

llogiq commented Jan 21, 2021

Sorry for leaving you waiting for so long. r=me once the conflicts are resolved.

@camsteffen
Copy link
Contributor Author

No worries!

r? @llogiq

^Is that right?

@llogiq
Copy link
Contributor

llogiq commented Jan 22, 2021

Thank you! @bors r+

@bors
Copy link
Contributor

bors commented Jan 22, 2021

📌 Commit 82bab19 has been approved by llogiq

@bors
Copy link
Contributor

bors commented Jan 22, 2021

⌛ Testing commit 82bab19 with merge 3c3f4a7...

@bors
Copy link
Contributor

bors commented Jan 22, 2021

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 3c3f4a7 to master...

@bors bors merged commit 3c3f4a7 into rust-lang:master Jan 22, 2021
@bors bors mentioned this pull request Jan 22, 2021
Closed
@giraffate giraffate mentioned this pull request Jan 26, 2021
Merged
@camsteffen camsteffen mentioned this pull request Apr 9, 2021
Merged
bors added a commit that referenced this pull request Apr 11, 2021
@bors
Auto merge of #7059 - camsteffen:filter-map, r=flip1995
a6b514c 
Deprecate `filter_map`

Since #6591, `filter_map` does not even lint `filter().map()`. The cases that are still linted make no sense IMO. So this just removes/deprecates it.

changelog: Deprecate `filter_map` lint

Closes #3424
Fixes #7050
@camsteffen camsteffen deleted the manual-filter-map branch July 8, 2021 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@llogiq llogiq

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make find_map lint be more conservative Make the filter_map lint more specific
4 participants
@camsteffen @rust-highfive @llogiq @bors