+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 > > > > >
