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 <dgurya...@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-requ...@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-requ...@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 __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
