don't ignore ignore (pun) after #6

[andreineculau]

#6 was incomplete, and so not fixing inaka/elvis#190

[elbrujohalcon]

@andreineculau can you rebase from master?

[elbrujohalcon]

@andreineculau If I understand your change correctly, you are now calling resolve_files/2 from resolve_files/1, which is what was missing to be able to process the ignore parameter.
But you're also removing a clause of resolve_files/1, in favor of using filter/1. That seems reasonable, but it changes the behaviour because before your change, if a config didn't include the filter property resolve_files/1 was calling elvis_file:find_files/2 (which is equivalent to elvis_file:find_files(Dirs, Pattern, recursive)) and now it will be calling elvis_file:find_files(Dirs, Pattern, local) instead.
I'm ok with that change since AFAIK it was an undocumented behaviour.
But, since you're rendering a function (find_files/2) and a piece of code (the part of find_files/3 that dealt with recursive in the 3rd parameter) useless, please refactor that module (i.e. remove the useless function, remove the useless 3rd parameter from the other function and all the places where it's called with local in there).
And, whenever possible, please add tests for these changes.

[elbrujohalcon]

@andreineculau the other option you have is to preserve the current behaviour and just call resolve_files/2 in both currently existing clauses of resolve_files/1 :)

[andreineculau]

On Fri, Mar 18, 2016 at 3:26 PM, Brujo Benavides notifications@github.com
wrote:

@andreineculau https://github.com/andreineculau can you present an
example of the problem you were seeing? Maybe as a test case?

Example: https://github.com/for-GET/katt with elvis-2.8.0 (not with the
elvis binary that is checked in)

andreineculau.com http://www.andreineculau.com

[elbrujohalcon]

@andreineculau I removed that comment because I figured out what was going on on my own later. Check my other comments, please.

[andreineculau]

It took me a while to understand what your point was :) that's why the delay.

So, are you certain you want it localonly? It's a shame that elvis_core didn't keep the history of elvis (rant: was it that hard to start from elvis, and massage it into becoming elvis_core? rather than start anew) but after some wasted time digging into the history, it looks like recursive was the default, and local was added later in inaka/elvis@be350e7

From where, the logic of the previous behaviour is hiding from me: search recursively if there's no filter, but search locally only (1 level) if there is a filter ? and also with the history, I would go with making resolve_files/1 call find_files/2 (recursive).

[elbrujohalcon]

I'm with you, @andreineculau I don't know what the reason for that recursive-only-if-no-filters behaviour was. I'll be delighted if you remove all that and I'll merge your PR with lots of happiness then :)

[andreineculau]

Update my PR... I didn't remove find_files/3 eventually. If I do that, then you end up with a less optimized version of the below in elvis_core.erl

    Dirname = filename:dirname(Path),
    Filename = filename:basename(Path),
    File = case elvis_file:find_files([Dirname], Filename, local) of
               [] -> throw({enoent, Path});
               [File0] -> File0
           end,

[elbrujohalcon]

I'll merge your PR and generate another one with my refactor idea :)

[andreineculau]

:s sorry, but thanks :)

On Fri, Mar 18, 2016 at 5:53 PM, Brujo Benavides notifications@github.com
wrote:

Merged #40 #40.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#40 (comment)

andreineculau.com http://www.andreineculau.com