You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@airflow.apache.org by Paul Rhodes <wi...@outlook.com> on 2016/06/02 21:46:27 UTC
PEP8, Linting and Code Smells
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
Re: PEP8, Linting and Code Smells
Posted by Jeremiah Lowin <jl...@apache.org>.
+2 I hate % so much!
On Thu, Jun 2, 2016 at 9:23 PM Chris Riccomini <cr...@apache.org>
wrote:
> @Maxime +1.. % based really annoys me.
>
> On Thu, Jun 2, 2016 at 6:13 PM, Maxime Beauchemin <
> maximebeauchemin@gmail.com> wrote:
>
> > 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 <cr...@apache.org>
> > 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 <
> > > maximebeauchemin@gmail.com> 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 <
> criccomini@apache.org
> > >
> > > > 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 <wi...@outlook.com>
> > > > 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 <be...@underyx.me>
> > > > > > 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 <withnale@outlook.com
> >
> > > > 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
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: PEP8, Linting and Code Smells
Posted by Chris Riccomini <cr...@apache.org>.
@Maxime +1.. % based really annoys me.
On Thu, Jun 2, 2016 at 6:13 PM, Maxime Beauchemin <
maximebeauchemin@gmail.com> wrote:
> 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 <cr...@apache.org>
> 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 <
> > maximebeauchemin@gmail.com> 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 <criccomini@apache.org
> >
> > > 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 <wi...@outlook.com>
> > > 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 <be...@underyx.me>
> > > > > 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 <wi...@outlook.com>
> > > 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
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: PEP8, Linting and Code Smells
Posted by Maxime Beauchemin <ma...@gmail.com>.
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 <cr...@apache.org>
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 <
> maximebeauchemin@gmail.com> 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 <cr...@apache.org>
> > 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 <wi...@outlook.com>
> > 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 <be...@underyx.me>
> > > > 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 <wi...@outlook.com>
> > 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
> > > > >
> > > >
> > >
> >
>
Re: PEP8, Linting and Code Smells
Posted by Chris Riccomini <cr...@apache.org>.
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 <
maximebeauchemin@gmail.com> 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 <cr...@apache.org>
> 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 <wi...@outlook.com>
> 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 <be...@underyx.me>
> > > 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 <wi...@outlook.com>
> 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
> > > >
> > >
> >
>
Re: PEP8, Linting and Code Smells
Posted by Maxime Beauchemin <ma...@gmail.com>.
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 <cr...@apache.org>
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 <wi...@outlook.com> 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 <be...@underyx.me>
> > 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 <wi...@outlook.com> 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
> > >
> >
>
Re: PEP8, Linting and Code Smells
Posted by Chris Riccomini <cr...@apache.org>.
@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 <wi...@outlook.com> 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 <be...@underyx.me>
> 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 <wi...@outlook.com> 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
> >
>
Re: PEP8, Linting and Code Smells
Posted by Paul Rhodes <wi...@outlook.com>.
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 <be...@underyx.me>
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 <wi...@outlook.com> 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
>
Re: PEP8, Linting and Code Smells
Posted by Bence Nagy <be...@underyx.me>.
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 <wi...@outlook.com> 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
>