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

Fix false positives on rule first_where #2638

Merged
merged 6 commits into from
Feb 27, 2019
Merged

Fix false positives on rule first_where #2638

merged 6 commits into from
Feb 27, 2019

Conversation

Jeehut
Copy link
Collaborator

@Jeehut Jeehut commented Feb 12, 2019

Fixes #1930.

@SwiftLintBot
Copy link

SwiftLintBot commented Feb 12, 2019

17 Messages
📖 Linting Aerial with this PR took 1.83s vs 1.82s on master (0% slower)
📖 Linting Alamofire with this PR took 4.04s vs 4.02s on master (0% slower)
📖 Linting Firefox with this PR took 12.81s vs 12.69s on master (0% slower)
📖 Linting Kickstarter with this PR took 0.22s vs 0.22s on master (0% slower)
📖 Linting Moya with this PR took 2.07s vs 2.03s on master (1% slower)
📖 Linting Nimble with this PR took 1.82s vs 1.84s on master (1% faster)
📖 Linting Quick with this PR took 0.61s vs 0.59s on master (3% slower)
📖 Linting Realm with this PR took 4.06s vs 3.5s on master (15% slower)
📖 Linting SourceKitten with this PR took 1.67s vs 1.19s on master (40% slower)
📖 Linting Sourcery with this PR took 5.19s vs 4.25s on master (22% slower)
📖 Linting Swift with this PR took 27.6s vs 27.59s on master (0% slower)
📖 Linting WordPress with this PR took 26.09s vs 24.7s on master (5% slower)
📖 This PR fixed a violation in Realm: /Users/vsts/agent/2.147.1/work/1/s/osscheck/Realm/RealmSwift/Tests/PerformanceTests.swift:327:21: warning: First Where Violation: Prefer using .first(where:) over .filter { }.first in collections. (first_where)
📖 This PR fixed a violation in Realm: /Users/vsts/agent/2.147.1/work/1/s/osscheck/Realm/RealmSwift/Tests/PerformanceTests.swift:341:21: warning: First Where Violation: Prefer using .first(where:) over .filter { }.first in collections. (first_where)
📖 This PR fixed a violation in Realm: /Users/vsts/agent/2.147.1/work/1/s/osscheck/Realm/RealmSwift/Tests/PerformanceTests.swift:358:17: warning: First Where Violation: Prefer using .first(where:) over .filter { }.first in collections. (first_where)
📖 This PR fixed a violation in Realm: /Users/vsts/agent/2.147.1/work/1/s/osscheck/Realm/RealmSwift/Tests/SwiftUnicodeTests.swift:36:20: warning: First Where Violation: Prefer using .first(where:) over .filter { }.first in collections. (first_where)
📖 This PR fixed a violation in Realm: /Users/vsts/agent/2.147.1/work/1/s/osscheck/Realm/RealmSwift/Tests/SwiftUnicodeTests.swift:54:20: warning: First Where Violation: Prefer using .first(where:) over .filter { }.first in collections. (first_where)

Generated by 🚫 Danger

@Jeehut Jeehut force-pushed the cg-first_where-fix branch from 905ea08 to d9113a7 Compare February 14, 2019 20:30
@Jeehut
Copy link
Collaborator Author

Jeehut commented Feb 25, 2019

@marcelofabri I think this can be merged, no? I fixed your comments quite a few days ago.

Copy link
Collaborator

@marcelofabri marcelofabri left a comment

Choose a reason for hiding this comment

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

Sorry, I thought I had approved this one

@Jeehut
Copy link
Collaborator Author

Jeehut commented Feb 27, 2019

No worries, thanks for approving! I'm not sure what the "perfect" procedure is here, can I just merge now given that you approved it and I myself don't see any reasons against merging? (Or to put it differently: Why didn't you merge right after approving it?)

@marcelofabri
Copy link
Collaborator

I didn't merge to make sure you had the final word on whether it's ready to merge as I know you have push access and can merge it.

Sometimes a PR looks good to merge but the original author may prefer to not merge it (e.g. my own PR here #2633).

Feel free to merge a PR whenever it's approved.

@Jeehut
Copy link
Collaborator Author

Jeehut commented Feb 27, 2019

Thanks for the explanation. Merging. 🎉

@Jeehut Jeehut merged commit 7155b86 into master Feb 27, 2019
@Jeehut Jeehut deleted the cg-first_where-fix branch February 27, 2019 11:25
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.

3 participants