The top 6 all seem totally do-able, and shouldn't be disruptive at all, provided small PRs and fast merges.
204 Style Line too long <https://landscape.io/github/apache/incubator-airflow/431/messages/72> 159 Style Import statements should be the first statements in a module <https://landscape.io/github/apache/incubator-airflow/431/messages/373> 91 Smell Unused argument <https://landscape.io/github/apache/incubator-airflow/431/messages/274> 76 Smell Use % formatting in logging functions but pass the % parameters as arguments <https://landscape.io/github/apache/incubator-airflow/431/messages/329> 66 Style Variable in function should be lowercase <https://landscape.io/github/apache/incubator-airflow/431/messages/89> 66 Smell Unused variable <https://landscape.io/github/apache/incubator-airflow/431/messages/273> On Thu, Jun 2, 2016 at 5:03 PM, Maxime Beauchemin < [email protected]> wrote: > This is definitely useful! I'd love to get in the 95%+ range. We may also > ease some of the rules eventually (it's easy to control in .landscape.yml) > but raw PEP8 is fine by me, though I allowed for up to 90 char per line, so > that we can use judgment there. > > Maybe the best approach is to start with "errors" where the solutions are > clear, and then proceed by a rule centric approach > <https://landscape.io/github/apache/incubator-airflow/431/messages>, it > makes the PRs easier to review. PRs with maybe 50 to 200 line edits seem > reasonable. > > All you need is a great soundtrack. I find doing that kind of mindless > repetitive work pretty enjoyable and relaxing. > > Max > > On Thu, Jun 2, 2016 at 4:04 PM, Chris Riccomini <[email protected]> > wrote: > > > @Paul, my opinion is that this is worth while as long as it isn't > > disruptive. I think the way to keep it from being disruptive is: > > > > 1. Small PRs > > 2. Only fix the issues that don't require refactoring (e.g. nit picking > on > > line length, spacing, etc) > > > > On Thu, Jun 2, 2016 at 4:01 PM, Paul Rhodes <[email protected]> > wrote: > > > > > http://landscape.io is already doing this for the project - it's > already > > > integrated with travis and run for every commit/PR > > > > > > > > > I was more concerned about improving the code quality - we already have > > > the metrics. > > > > > > > > > ________________________________ > > > From: Bence Nagy <[email protected]> > > > Sent: 02 June 2016 23:52 > > > To: Airflow Dev > > > Subject: Re: PEP8, Linting and Code Smells > > > > > > Hey, > > > > > > Please check out http://coala-analyzer.org for static analysis, it's > > > awesome! http://gitmate.com is able to run these checks automatically > > for > > > PRs. See https://github.com/coala-analyzer/coala/pull/2220 for a live > > > example (hover over the green checkmark next to the commit hash). > > > > > > I'd love to see as many things copied from coala's CI system as > possible > > - > > > they even have automatic pypi prerelease version deployments for each > > > merge! > > > > > > On Thu, Jun 2, 2016 at 2:46 PM Paul Rhodes <[email protected]> > wrote: > > > > > > > Hi > > > > > > > > > > > > I've asked a few of the committers about this but thought I'd ping > this > > > to > > > > the list. > > > > > > > > > > > > We've integrated landscape.io to do automated code checks and as of > > > right > > > > now it's flagging 8 Errors, 416 Smells and 784 Style alerts. I've > had a > > > > look at some of these and thought I might pick off some of the lower > > > > hanging fruit in this area, but I'd just like to understand if > > improving > > > > the static code scores is seen to be of value right now... > > > > > > > > > > > > I'd like to see the scores improve, but like the testing and the > > changes > > > > to model.py, it's a big job which touches a significant number of > lines > > > of > > > > code. > > > > > > > > > > > > Of course, it's also fine if we choose to keep the codebase as-is but > > if > > > > that's the case we should adjust .landscape.yml to a profile that is > > > > tailored to alert on only the things we care about. (For example, I > > would > > > > be in favour of allowing _(\w+) in the pylint dummy variable syntax > to > > > > allow context to be maintained for dummy variables)) > > > > > > > > > > > > cheers > > > > > > > > > > > > Paul > > > > > > > > > >
