Let's wire a custom a linter command that can be called locally and respect an agreed upon set of parameters (pylint + config file, based off of our current .landscape.yml ).
flake8 is far from being as good as pylint and can't be customized much AFAICT, but variations on the command bellow can help you lint [only] your PR: `git diff HEAD^ | flake8 --diff` It's a good thing to integrate in your workflow until we get an equivalent pylint command/config On Thu, Mar 16, 2017 at 5:03 PM, Alex Guziel <[email protected] > wrote: > +1 also > > We have code review already and the amount of false positives makes this > useless. > > On Thu, Mar 16, 2017 at 5:02 PM, Maxime Beauchemin < > [email protected]> wrote: > > > +1 as well > > > > I'm disappointed because the service is inches away from getting > everything > > right. As Bolke said, behind the cover it's little more than pylint, git > > hooks, and a somewhat-fancy ui. > > > > Operationally it's been getting in the way. > > > > There's a way to pipe the output of git diff into pylint and check > whether > > the touched lines need linting, in which case we should break the build. > > This could run in it's own slot in the Travis build matrix. > > > > Max > > > > On Thu, Mar 16, 2017 at 4:51 PM, Bolke de Bruin <[email protected]> > wrote: > > > > > We can do it in Travis’ afaik. We should replace it. > > > > > > So +1. > > > > > > B. > > > > > > > On 16 Mar 2017, at 16:48, Jeremiah Lowin <[email protected]> wrote: > > > > > > > > This may be an unpopular opinion, but most Airflow PRs have a little > > red > > > > "x" next to them not because they have failing unit tests, but > because > > > the > > > > Landscape check has decided they introduce bad code. > > > > > > > > Unfortunately Landscape is often wrong -- here it is telling me my > > latest > > > > PR introduced no less than 30 errors... in files I didn't touch! > > > > https://github.com/apache/incubator-airflow/pull/2157 (however, it > > > gives me > > > > credit for fixing 23 errors in those same files, so I've got that > going > > > for > > > > me... which is nice.) > > > > > > > > The upshot is that Github's "health" indicator can be swayed by minor > > or > > > > erroneous issues, and therefore it serves little purpose other than > > > making > > > > it look like every PR is bad. This creates committer fatigue, since > > every > > > > PR needs to be parsed to see if it actually is OK or not. > > > > > > > > Don't get me wrong, I'm all for proper style and on occasion > Landscape > > > has > > > > pointed out problems that I've gone and fixed. But on the whole, I > > > believe > > > > that having it as part of our red / green PR evaluation -- equal to > and > > > > often superseding unit tests -- is harmful. I'd much rather be able > to > > > scan > > > > the PR list and know unequivocally that "green" indicates ready to > > merge. > > > > > > > > J > > > > > > > > >
