bradleyfalzon / revgrep

Filters output from static analysis tools, showing only recently changed lines of code
Apache License 2.0
103 stars 15 forks source link

Fix case when changed file has deletions only: it was treated as full… #11

Closed jirfag closed 7 years ago

jirfag commented 7 years ago

…y changed

bradleyfalzon commented 7 years ago

Thanks @jirfag, this is very interesting timing, I've just fixed this in a slightly different way in #12.

I do appreciate the contribution, but as you would see in the failed tests, the proposed change here isn't quite enough. Removing the || fchanges == nil meant that issues in new files (untracked files) would not be included, and that's the reason those tests are failing.

But you were on the right track, in #12 I've changed it so files with just deletions has a non-nil (but still empty) slice. It's not the best way, but it'll do for now.

Thanks for the contribution, but I think #12 should cover it.

jirfag commented 7 years ago

ok, when can I expect #12 to be merged into master?

bradleyfalzon commented 7 years ago

I'll do it within the next 18 hours, likely less. I usually like to sit on it for a short period before I merge, just in case another idea or someone else has another idea.

bradleyfalzon commented 7 years ago

@jirfag I've merged the branch and tagged a new release, let me know if this doesn't resolve your original issue. Thanks again.