+1 as well though I have found it useful on larger PRs to help me catch some issues so it probably makes sense to make to add the travis linting at the same time we remove it in landscape. Not sure how much usability we lose by losing the landscape UI but I like that all of the errors would be in one place.
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 > >
