timvideos / edid.tv

EDID Database Website
GNU General Public License v2.0
26 stars 7 forks source link

Include all Python files recursively again in Travis pylint #30

Closed pp3345 closed 5 years ago

pp3345 commented 5 years ago

This implements a variation of @mithro's suggestion from https://github.com/timvideos/edid.tv/pull/28

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 99.187% when pulling b2b6a633ac80c446a5d2af46b629ca0de4a2444c on pp3345:travis-pylint-all-files into de8ecff2cdbc7f5137f80445d7dc659b3ffbe9b3 on timvideos:master.

pp3345 commented 5 years ago

This is probably the ugliest bash command I've ever written, but it works :man_shrugging:

pp3345 commented 5 years ago

Looking at this again, I am an idiot. Here is a much simpler and saner solution.

For reference, the previous approach was

pylint --reports=n *.py $(find -type d \( -path "./frontend*" -o -path "./edid_parser*" \) -exec sh -c 'ls -1 {}/*.py 2> /dev/null | (test $(wc -l) -gt 0 && echo {})' ';' | awk '{ print $1 "/*.py" }')
mithro commented 5 years ago

I think you want something like this

$(find ./frontend* ./edid_parser* -type f -and -name \*.py -and -not -empty)
pp3345 commented 5 years ago

I think the current solution is okay, what do you think? Requiring shopt is a bit unfurtonate, but apart from that... If you prefer using find I am okay with that, too, of course.

mithro commented 5 years ago

One find command is superior to one find command + ls + awk...

mithro commented 5 years ago

Oh - I didn't see the shopt change...