Sounds like a probable consensus. I think this is a good time for me to admit I have no idea how to turn it off or move it to Travis.
Could anyone please volunteer to take that on? On Thu, Mar 16, 2017 at 8:11 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 > > > > > > > > >
