Side note I think I want to remove "Smell Use % formatting in logging functions but pass the % parameters as arguments". `%` formatting is outdated and folks are moving away from it in favor of "".format() .
On Thu, Jun 2, 2016 at 5:21 PM, Chris Riccomini <[email protected]> wrote: > 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 > > > > > > > > > > > > > > >
