Open Stack

On 04/04/2016 07:05 AM, Matthew Mosesohn wrote:
> Hi Dmitry,
>
> I've seen several cases where core reviewers bully contributors into
> refactoring a particular piece of logic because it contains common
> lines relating to some non-ideal code, even if the change doesn't
> relate to this logic.
> In general, I'm ok with formatting issues, but changing how a piece of
> existing code works is over the line. It should be handled as a
> separate bug.
>
> But yes, in general, if someone complains about something unrelated to
> your patch, he or she should just file a bug with what is required.
>
> -Matthew
>
>
> On Mon, Apr 4, 2016 at 3:46 PM, Dmitry Guryanov <dguryanov at mirantis.com> wrote:
> > Hello, colleagues!
> >
> > It's often not so easy to decide, if you should include some unrelated
> > changes to your patch, like fixing spaces, renaming variables or something
> > else, which don't change logic. On the one hand you see something's wrong
> > with the code and you'd like to fix it, on the other hand reviewers can vote
> > of -1 and you'll have to fix you patch and upload it again and this is very
> > annoying. You can also create separate review for such changes, but it will
> > require additional effort from you and reviewers.
> >
> > If you are a reviewer, and you've noted unrelated changes you may hesitate,
> > if you should ask an author to remove them and upload new version of the
> > patch or not. Also such extra changes may confuse you sometimes.
> >
> > So I suggest creating separate patches for unrelated changes if they add new
> > chucks to patch. And I'd like to ask authors to clearly state in the subject
> > of a commit message, that this patch just fixes formatting. And reviewers
> > shouldn't check such patches too severely, so that they'll get into repo as
> > soon as possible.
> >
> > What do you think?
> >
> >
> > --
> > Dmitry Guryanov
> >
> > __________________________________________________________________________
> > OpenStack Development Mailing List (not for usage questions)
> > Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> >
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
I agree with Matthew, but huge +1 to separate patch/bug for formatting/whitespace issues.

-- 
Jason E. Rist
Senior Software Engineer
OpenStack User Interfaces
Red Hat, Inc.
openuc: +1.972.707.6408
mobile: +1.720.256.3933
Freenode: jrist
github/twitter: knowncitizen