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

Rewrite ConditionalBindingCascadeRule #620

Merged
merged 9 commits into from
Apr 16, 2016

Conversation

norio-nomura
Copy link
Collaborator

@norio-nomura norio-nomura force-pushed the nn-rewrite-conditional_binding_cascade branch from d726da1 to d5e0a90 Compare April 15, 2016 23:38
@norio-nomura norio-nomura changed the title [WIP] Rewrite ConditionalBindingCascadeRule Rewrite ConditionalBindingCascadeRule Apr 15, 2016
@norio-nomura
Copy link
Collaborator Author

Updated.

@norio-nomura
Copy link
Collaborator Author

make sum_test fails randomly on both Travis-CI and local mac.
We need to update SourceKitten in Package.swift, but I won't update it in this PR.

@@ -15,6 +15,10 @@
* SwiftLint no longer crashes when SourceKitService crashes.
[Norio Nomura](https://github.com/norio-nomura)

* Rewrite `conditional_binding_cascade` rule.
[Norio Nomura](https://github.com/norio-nomura)
[#617](https://github.com/jpsim/SwiftLint/issues/617)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is the wrong url

@jpsim
Copy link
Collaborator

jpsim commented Apr 16, 2016

👍 other than where noted

@norio-nomura
Copy link
Collaborator Author

Performance note:
The duration of ConditionalBindingCascadeRule on linting Carthage-16.2 increased from 64ms (w/ 62c7d4a) to 283ms (w/ d3f698f).

@jpsim
Copy link
Collaborator

jpsim commented Apr 16, 2016

Looks great to me, thanks!

@jpsim jpsim merged commit 3b96daf into master Apr 16, 2016
@jpsim jpsim deleted the nn-rewrite-conditional_binding_cascade branch April 16, 2016 08:05
@norio-nomura
Copy link
Collaborator Author

Thanks for reviews and merging! 🙏

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