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