You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@qpid.apache.org by Jiri Daněk <jd...@redhat.com> on 2020/11/05 15:32:05 UTC

Autopep8 for DISPATCH-1814

Hello folks,

(https://issues.apache.org/jira/browse/DISPATCH-1814 Apply autofixes to
resolve some flake8 code formatting issues)

I have prior positive experience with autopep8,
https://pypi.org/project/autopep8/. It is a tool to automatically reformat
Python source code. It can either selectively reformat to fix only a
specific flake8 warning, or it can just do it all in one go.

What do you think about running this on the Qpid Proton and Qpid Dispatch
code? Is there a good time when to do it? Would it be better to fix each
warning individually, in its own commit (to simplify manual review), or do
it all in one commit (to simplify git history, and spend less time on it)?

I am personally in favour of a single commit in which to do all the
whitespace changes in one go. I've always found autopep8 to work reliably.

Regarding potential issues, whitespace changes would affect ongoing work in
progress (although the autoformatter can be run on the PRs as well), so it
seems to me that a good time to land this would be after a release.
-- 
Mit freundlichen Grüßen / Kind regards
Jiri Daněk

Re: Autopep8 for DISPATCH-1814

Posted by Jiri Daněk <jd...@redhat.com>.
On Fri, Nov 6, 2020 at 3:15 PM Ken Giusti <kg...@redhat.com> wrote:

> On Fri, Nov 6, 2020 at 5:40 AM Robbie Gemmell <ro...@gmail.com>
> wrote:
>
> > On Thu, 5 Nov 2020 at 15:32, Jiri Daněk <jd...@redhat.com> wrote:
> > >
> > > Hello folks,
> > >
> > > (https://issues.apache.org/jira/browse/DISPATCH-1814 Apply autofixes
> to
> > > resolve some flake8 code formatting issues)
> > >
> > > I have prior positive experience with autopep8,
> > > https://pypi.org/project/autopep8/. It is a tool to automatically
> > reformat
> > > Python source code. It can either selectively reformat to fix only a
> > > specific flake8 warning, or it can just do it all in one go.
> > >
> > > What do you think about running this on the Qpid Proton and Qpid
> Dispatch
> > > code? Is there a good time when to do it? Would it be better to fix
> each
> > > warning individually, in its own commit (to simplify manual review), or
> > do
> > > it all in one commit (to simplify git history, and spend less time on
> > it)?
> > >
> > > I am personally in favour of a single commit in which to do all the
> > > whitespace changes in one go. I've always found autopep8 to work
> > reliably.
> > >
> > > Regarding potential issues, whitespace changes would affect ongoing
> work
> > in
> > > progress (although the autoformatter can be run on the PRs as well), so
> > it
> > > seems to me that a good time to land this would be after a release.
> > > --
> > > Mit freundlichen Grüßen / Kind regards
> > > Jiri Daněk
> >
> > I'll leave 'what to do?' position to the folks with more knowledge /
> > actually working on the bits, but I think you nailed the answer to
> > 'when to do it?' aspect already: if doing anything like this an agreed
> > point just after a release seems to be clearly the best time for such
> > things.
> >
> > Robbie
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
> > For additional commands, e-mail: users-help@qpid.apache.org
> >
> >
> I agree with Robbie in terms of when (shortly after a new release).
>
> I'm in favor of doing this and would prefer a single commit if possible.
> -K
>

Autopep8 fixes are applied now for Proton and Dispatch, in
https://issues.apache.org/jira/browse/PROTON-2320 and
https://issues.apache.org/jira/browse/DISPATCH-1814. In case of dispatch I
decided not to apply the autofixes which would remove vertical alignment in
Python sources. (Removal of multiple spaces before/after a comma, colon,
etc.)

There are follow-up jiras for reducing the number of flake8 suppressions in
tox.ini, https://issues.apache.org/jira/browse/PROTON-2322 and
https://issues.apache.org/jira/browse/DISPATCH-1974.

I decided not to do this for the Qpid Cpp Broker, because the releases
there are infrequent.
-- 
Mit freundlichen Grüßen / Kind regards
Jiri Daněk

Re: Autopep8 for DISPATCH-1814

Posted by Ken Giusti <kg...@redhat.com>.
On Fri, Nov 6, 2020 at 5:40 AM Robbie Gemmell <ro...@gmail.com>
wrote:

> On Thu, 5 Nov 2020 at 15:32, Jiri Daněk <jd...@redhat.com> wrote:
> >
> > Hello folks,
> >
> > (https://issues.apache.org/jira/browse/DISPATCH-1814 Apply autofixes to
> > resolve some flake8 code formatting issues)
> >
> > I have prior positive experience with autopep8,
> > https://pypi.org/project/autopep8/. It is a tool to automatically
> reformat
> > Python source code. It can either selectively reformat to fix only a
> > specific flake8 warning, or it can just do it all in one go.
> >
> > What do you think about running this on the Qpid Proton and Qpid Dispatch
> > code? Is there a good time when to do it? Would it be better to fix each
> > warning individually, in its own commit (to simplify manual review), or
> do
> > it all in one commit (to simplify git history, and spend less time on
> it)?
> >
> > I am personally in favour of a single commit in which to do all the
> > whitespace changes in one go. I've always found autopep8 to work
> reliably.
> >
> > Regarding potential issues, whitespace changes would affect ongoing work
> in
> > progress (although the autoformatter can be run on the PRs as well), so
> it
> > seems to me that a good time to land this would be after a release.
> > --
> > Mit freundlichen Grüßen / Kind regards
> > Jiri Daněk
>
> I'll leave 'what to do?' position to the folks with more knowledge /
> actually working on the bits, but I think you nailed the answer to
> 'when to do it?' aspect already: if doing anything like this an agreed
> point just after a release seems to be clearly the best time for such
> things.
>
> Robbie
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
> For additional commands, e-mail: users-help@qpid.apache.org
>
>
I agree with Robbie in terms of when (shortly after a new release).

I'm in favor of doing this and would prefer a single commit if possible.
-K

Re: Autopep8 for DISPATCH-1814

Posted by Robbie Gemmell <ro...@gmail.com>.
On Thu, 5 Nov 2020 at 15:32, Jiri Daněk <jd...@redhat.com> wrote:
>
> Hello folks,
>
> (https://issues.apache.org/jira/browse/DISPATCH-1814 Apply autofixes to
> resolve some flake8 code formatting issues)
>
> I have prior positive experience with autopep8,
> https://pypi.org/project/autopep8/. It is a tool to automatically reformat
> Python source code. It can either selectively reformat to fix only a
> specific flake8 warning, or it can just do it all in one go.
>
> What do you think about running this on the Qpid Proton and Qpid Dispatch
> code? Is there a good time when to do it? Would it be better to fix each
> warning individually, in its own commit (to simplify manual review), or do
> it all in one commit (to simplify git history, and spend less time on it)?
>
> I am personally in favour of a single commit in which to do all the
> whitespace changes in one go. I've always found autopep8 to work reliably.
>
> Regarding potential issues, whitespace changes would affect ongoing work in
> progress (although the autoformatter can be run on the PRs as well), so it
> seems to me that a good time to land this would be after a release.
> --
> Mit freundlichen Grüßen / Kind regards
> Jiri Daněk

I'll leave 'what to do?' position to the folks with more knowledge /
actually working on the bits, but I think you nailed the answer to
'when to do it?' aspect already: if doing anything like this an agreed
point just after a release seems to be clearly the best time for such
things.

Robbie

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org