On 26/09, Morgan Fainberg wrote: > As a core (and former PTL) I just ignored commit message -1s unless there is > something majorly wrong (no bug id where one is needed, etc). > > I appreciate well formatted commits, but can we let this one go? This > discussion is so far into the meta-bike-shedding (bike shedding about bike > shedding commit messages) ... If a commit message is *that* bad a -1 (or just > fixing it?) Might be worth it. However, if a commit isn't missing key info > (bug id? Bp? Etc) and isn't one long incredibly unbroken sentence moving from > topic to topic, there isn't a good reason to block the review. > > It is not worth having a bot -1 bad commits or even having gerrit muck with > them. Let's do the job of the reviewer and actually review code instead of > going crazy with commit messages. > > Sent via mobile >
I have to disagree, as reviewers we have to make sure that guidelines are followed, if we have an explicit guideline that states that the limit length is 72 chars, I will -1 any patch that doesn't follow the guideline, just as I would do with i18n guideline violations. Typos are a completely different matter and they should not be grouped together with guideline infringements. I agree that it is a waste of time and resources when you have to -1 a patch for this, but there multiple solutions, you can make sure your editor does auto wrapping at the right length (I have mine configured this way), or create a git-enforce policy with a client-side hook, or do like Ihar is trying to do and push for a guideline change. I don't mind changing the guideline to any other length, but as long as it is 72 chars I will keep enforcing it, as it is not the place of reviewers to decide which guidelines are worthy of being enforced and which ones are not. Cheers, Gorka. > > On Sep 26, 2015, at 21:19, Ian Wells <ijw.ubu...@cack.org.uk> wrote: > > > > Can I ask a different question - could we reject a few simple-to-check > > things on the push, like bad commit messages? For things that take 2 > > seconds to fix and do make people's lives better, it's not that they're > > rejected, it's that the whole rejection cycle via gerrit review (push/wait > > for tests to run/check website/swear/find change/fix/push again) is out of > > proportion to the effort taken to fix it. > > > > It seems here that there's benefit to 72 line messages - not that everyone > > sees that benefit, but it is present - but it doesn't outweigh the current > > cost. > > -- > > Ian. > > > > > >> On 25 September 2015 at 12:02, Jeremy Stanley <fu...@yuggoth.org> wrote: > >> On 2015-09-25 16:15:15 +0000 (+0000), Fox, Kevin M wrote: > >> > Another option... why are we wasting time on something that a > >> > computer can handle? Why not just let the line length be infinite > >> > in the commit message and have gerrit wrap it to <insert random > >> > number here> length lines on merge? > >> > >> The commit message content (including whitespace/formatting) is part > >> of the data fed into the hash algorithm to generate the commit > >> identifier. If Gerrit changed the commit message at upload, that > >> would alter the Git SHA compared to your local copy of the same > >> commit. This quickly goes down a Git madness rabbit hole (not the > >> least of which is that it would completely break signed commits). > >> -- > >> Jeremy Stanley > >> > >> __________________________________________________________________________ > >> 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 > __________________________________________________________________________ > 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
