[openstack-dev] [fuel] Unrelated changes in patches
mmosesohn at mirantis.com
Mon Apr 4 13:05:28 UTC 2016
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
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.
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
More information about the OpenStack-dev