search

0k / shyaml

YAML for command line
BSD 2-Clause "Simplified" License
767 stars 57 forks source link

Introduce conformant pycodestyle #45

Closed szepeviktor closed 6 years ago

szepeviktor commented 6 years ago

@vaab Reborn #44

szepeviktor commented 6 years ago

https://travis-ci.org/0k/shyaml/jobs/369794311 For E722 do not use bare 'except' we have two ways:

  1. change to except TypeError:
  2. ignore it: ignore = E266,E722

Please advise.

szepeviktor commented 6 years ago

@vaab Could you decide on the last finding?

szepeviktor commented 6 years ago

@vaab Is it OK?

vaab commented 6 years ago

Hi, I have had a look to your changes and I'm integrating them with some minor exception (and I solved the pycodestyle warning about except by catching except Exception). I'm not willing to integrate the conformity check in travis tests however. I understand that this is a perfectly valid way to do it, but I do want to separate concerns here. The conformity tests do not need to be checked on each platform and they have their place in other tests that I have on my end and that are run prior to any platform tests.

vaab commented 6 years ago

Is it all okay for you ? Feel free to re-open if there's anything that I missed.

szepeviktor commented 6 years ago

It is OK for me as it is your project. However when developing CI/CD for my clients I propose automated checks in CI.

I've sent this PR as this is a community project and pycodestyle will help spotting errors.

vaab commented 6 years ago

Well, you should consider not mixing style check with test check in your CI. You are wasting time, computer power and energy, and it is inefficient. There are no reason to check the style in your platform matrix (your python style is good or not, this should not be tested over and over on windows, linux, python 2.7, 3.4, 3.5 ...). I'm not saying you should not put some style checks... I use and have used flake8, pep8 (old name of pycodestyle), pycheckers, pylint... . They have their place in CI, or pre-commit as I do, where you are coding (in your editor preferably, as you type)... I don't want to run full tests to have the result of coding style checking come so late, they should be instant. If you want absolutely in a external CI loop, there are probably other webservices than travis that will do this check... once.

In travis hooks, you are doing it wrong, IMHO. But diversity of view is interesting, and if you have argumented views on why putting it in travis is a good idea (it is NOT a very bad idea, it is just not good enough in my settings for the reason given before), I'm very happy to hear your arguments.

szepeviktor commented 6 years ago

... once

I agree. We could change "script:" in "matrix:" and run 1 style check.

It is up to you.