search

ashfurrow / danger-ruby-swiftlint

A Danger plugin for SwiftLint.
https://rubygems.org/gems/danger-swiftlint
MIT License
203 stars 81 forks source link

Performance enhancement when `lint_all_files` is true. #131

Closed ra1028 closed 5 years ago

ra1028 commented 5 years ago

Hi @ashfurrow , Thank you for useful Danger plugin.

Description

In my situation, the git diff with master is large, the time taken for lint via Danger is greatly increased on CI. I used the lint_all_files option, but I didn't get the expected results. This caused by the processing time for searching target files from git diff and repository in the internal of danger-ruby-swiftlint. I fixed it with omitting process only when lint_all_files istrue.

For example, in my case, took 7 minutes for the linting on the CI before the fix, it has been reduced to less than 20 seconds now.

And one more thing, I propose to set the default value of lint_all_files to true.

ashfurrow commented 5 years ago

I pulled locally and verified the specs still pass – I fixed the underlying problem here: https://github.com/ashfurrow/danger-ruby-swiftlint/commit/9080cd6a2a555294a7fc55a6951fdbc0831b16cb

ashfurrow commented 5 years ago

Okay, this was released as 0.21.1. Thanks again!

ra1028 commented 5 years ago

the problem was that find_swift_files was taking too long based on the git diff?

Yup, the algorithm on find_swift_files needs to increase processing speed in the future.

I'd prefer to keep lint_all_files set to false by default, since if a PR only changes file A, we wouldn't want warnings from file B. What do you think?

Agreed.

@ashfurrow Thank you for quick response! Keep up the great work.