ashfurrow / danger-ruby-swiftlint

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

inline_mode doesn't work with fail_on_error #91

Closed omirho closed 6 years ago

omirho commented 6 years ago

Add the following line in your Dangerfile

swiftlint.lint_files inline_mode: true, fail_on_error: false

Now introduce lint errors in your code.

Danger will fail regardless of the value of fail_on_error displaying the lint errors in the PR. The culprit is this line.

I don't know if this is the intended behavior or not. Either the bug should be fixed or the README should be updated to highlight this fact.

Will be happy to raise a PR in either case.

omirho commented 6 years ago

If the intended behavior is to respect fail_on_error, then the linter errors will have to be treated as warnings in inline_mode.

This unexpected behavior was introduced with this PR.

ashfurrow commented 6 years ago

Yeah, looks like Rubocop may have replaced the fail with raise. I think moving it back should fix the bug? In any event, if you or someone else has the time, I'd appreciate a PR :bow:

omirho commented 6 years ago

@ashfurrow This is something different than what you mention. fail_on_error is not respected with inline_mode. Both are written in mutually exclusive blocks.

I'll be happy to raise a PR, but I don't know what the intended behavior should be.

ashfurrow commented 6 years ago

Ahhhhh I see. inline_mode was added in #29, I wonder if @leonhartX could offer some perspective on how it should work with fail_on_error?

leonhartX commented 6 years ago

Yes, I think it's better to respect the fail_on_error, maybe change the fail to warn when fail_on_error set to false?

ashfurrow commented 6 years ago

@leonhartX thanks for chiming in!

omirho commented 6 years ago

Cool! I'll raise a PR this weekend. I'll start working on the weekend, so if anyone else wants to pick this up. Feel free to self-assign. :)