[openstack-dev] [all] -1 due to line length violation in commit messages

Miguel Angel Ajo mangelajo at redhat.com
Mon Sep 28 09:53:51 UTC 2015


IMO those checks should be automated, as we automate pep8 checks on our
python code.

I agree, that if we have a rule for this, we should follow it, but it's 
a waste
of time reviewing and enforcing this manually.

Best regards.
Miguel Ángel.

Gorka Eguileor wrote:
> 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.ubuntu at 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<fungi at 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-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
>
>> __________________________________________________________________________
>> 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




More information about the OpenStack-dev mailing list