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

2015-10-07 Thread Matthew Booth
On Fri, Sep 25, 2015 at 3:44 PM, Ihar Hrachyshka 
wrote:

> Hi all,
>
> releases are approaching, so it’s the right time to start some bike
> shedding on the mailing list.
>
> Recently I got pointed out several times [1][2] that I violate our commit
> message requirement [3] for the message lines that says: "Subsequent lines
> should be wrapped at 72 characters.”
>
> I agree that very long commit message lines can be bad, f.e. if they are
> 200+ chars. But <= 79 chars?.. Don’t think so. Especially since we have 79
> chars limit for the code.
>
> We had a check for the line lengths in openstack-dev/hacking before but it
> was killed [4] as per openstack-dev@ discussion [5].
>
> I believe commit message lines of <=80 chars are absolutely fine and
> should not get -1 treatment. I propose to raise the limit for the guideline
> on wiki accordingly.
>

IIUC, the lower limit for commit messages is because git displays them
indented by default, which means that lines which are 80 chars long will
wrap on a display which is 80 chars wide. I personally use terminal windows
which are 80 chars wide, and I do find long lines in commit messages
annoying, so I'm personally in favour of retaining the lower limit. Can't
say I'd storm any castles if it was changed, but if most people use git the
way I do[1] I guess it should stay.

Matt

[1] I have no idea if this is the case.
__
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


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

2015-10-05 Thread Steve Baker

On 06/10/15 06:23, Ben Nemec wrote:

On 10/05/2015 12:12 PM, Jeremy Stanley wrote:

On 2015-10-05 12:00:31 -0500 (-0500), Ben Nemec wrote:
[...]

Note that it's best to do this once the change is ready to be
approved. If you do it earlier and the committer pushes a new
patch set without fixing the commit message, it will revert the
fix made through the web interface.

Well, one workflow tweak which avoids that is to always pull the
latest state of your change from Gerrit before you start modifying
it rather than assuming what is in your filesystem is still current.
It also helps to check Gerrit before pushing a new patchset (or lurk
in an IRC channel where the openstackgerrit bot reports uploads for
that repo), making sure nobody else has updated that change while
you were editing.

That said, a lot of developers probably don't do this.


Yeah, and I'm assuming we shouldn't need to fix commit messages much for
experienced developers who know to do this, but maybe I'm being
optimistic there. :-)

What I generally do is either edit the commit message through gerrit 
just before approving the change, or leave a review comment requesting 
the eventual approver to edit the commit message


__
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


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

2015-10-05 Thread Ben Nemec
On 10/01/2015 02:56 AM, Ghe Rivero wrote:
> If anyone disagrees with the commit format, please, go ahead and fix it (It's
> really easy using the gerrit web) For such cosmetic changes (and others
> similars), we should not wait for the author to do it. Sometimes, for a stupid
> comma, and with all the TZ, a change can need more than a day to be fixed and
> approved.

Note that it's best to do this once the change is ready to be approved.
 If you do it earlier and the committer pushes a new patch set without
fixing the commit message, it will revert the fix made through the web
interface.

> 
> Ghe Rivero
> 
> Quoting Ihar Hrachyshka (2015-09-29 18:05:37)
>>> On 25 Sep 2015, at 16:44, Ihar Hrachyshka  wrote:
>>>
>>> Hi all,
>>>
>>> releases are approaching, so it’s the right time to start some bike 
>>> shedding on the mailing list.
>>>
>>> Recently I got pointed out several times [1][2] that I violate our commit 
>>> message requirement [3] for the message lines that says: "Subsequent lines 
>>> should be wrapped at 72 characters.”
>>>
>>> I agree that very long commit message lines can be bad, f.e. if they are 
>>> 200+ chars. But <= 79 chars?.. Don’t think so. Especially since we have 79 
>>> chars limit for the code.
>>>
>>> We had a check for the line lengths in openstack-dev/hacking before but it 
>>> was killed [4] as per openstack-dev@ discussion [5].
>>>
>>> I believe commit message lines of <=80 chars are absolutely fine and should 
>>> not get -1 treatment. I propose to raise the limit for the guideline on 
>>> wiki accordingly.
>>>
>>> Comments?
>>>
>>> [1]: https://review.openstack.org/#/c/224728/6//COMMIT_MSG
>>> [2]: https://review.openstack.org/#/c/227319/2//COMMIT_MSG
>>> [3]: 
>>> https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure
>>> [4]: https://review.openstack.org/#/c/142585/
>>> [5]: 
>>> http://lists.openstack.org/pipermail/openstack-dev/2014-December/thread.html#52519
>>>
>>> Ihar
>>
>> Thanks everyone for replies.
>>
>> Now I realize WHY we do it with 72 chars and not 80 chars (git log output). 
>> :) I updated the wiki page with how to configure Vim to enforce the rule. I 
>> also removed the notion of gating on commit messages because we have them 
>> removed since recently.
>>
>> Ihar
>>
>>
>> __
>> 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


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

2015-10-05 Thread Ben Nemec
On 10/05/2015 12:12 PM, Jeremy Stanley wrote:
> On 2015-10-05 12:00:31 -0500 (-0500), Ben Nemec wrote:
> [...]
>> Note that it's best to do this once the change is ready to be
>> approved. If you do it earlier and the committer pushes a new
>> patch set without fixing the commit message, it will revert the
>> fix made through the web interface.
> 
> Well, one workflow tweak which avoids that is to always pull the
> latest state of your change from Gerrit before you start modifying
> it rather than assuming what is in your filesystem is still current.
> It also helps to check Gerrit before pushing a new patchset (or lurk
> in an IRC channel where the openstackgerrit bot reports uploads for
> that repo), making sure nobody else has updated that change while
> you were editing.
> 
> That said, a lot of developers probably don't do this.
> 

Yeah, and I'm assuming we shouldn't need to fix commit messages much for
experienced developers who know to do this, but maybe I'm being
optimistic there. :-)

__
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


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

2015-10-05 Thread Jeremy Stanley
On 2015-10-05 12:00:31 -0500 (-0500), Ben Nemec wrote:
[...]
> Note that it's best to do this once the change is ready to be
> approved. If you do it earlier and the committer pushes a new
> patch set without fixing the commit message, it will revert the
> fix made through the web interface.

Well, one workflow tweak which avoids that is to always pull the
latest state of your change from Gerrit before you start modifying
it rather than assuming what is in your filesystem is still current.
It also helps to check Gerrit before pushing a new patchset (or lurk
in an IRC channel where the openstackgerrit bot reports uploads for
that repo), making sure nobody else has updated that change while
you were editing.

That said, a lot of developers probably don't do this.
-- 
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


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

2015-10-01 Thread Ghe Rivero
If anyone disagrees with the commit format, please, go ahead and fix it (It's
really easy using the gerrit web) For such cosmetic changes (and others
similars), we should not wait for the author to do it. Sometimes, for a stupid
comma, and with all the TZ, a change can need more than a day to be fixed and
approved.

Ghe Rivero

Quoting Ihar Hrachyshka (2015-09-29 18:05:37)
> > On 25 Sep 2015, at 16:44, Ihar Hrachyshka  wrote:
> >
> > Hi all,
> >
> > releases are approaching, so it’s the right time to start some bike 
> > shedding on the mailing list.
> >
> > Recently I got pointed out several times [1][2] that I violate our commit 
> > message requirement [3] for the message lines that says: "Subsequent lines 
> > should be wrapped at 72 characters.”
> >
> > I agree that very long commit message lines can be bad, f.e. if they are 
> > 200+ chars. But <= 79 chars?.. Don’t think so. Especially since we have 79 
> > chars limit for the code.
> >
> > We had a check for the line lengths in openstack-dev/hacking before but it 
> > was killed [4] as per openstack-dev@ discussion [5].
> >
> > I believe commit message lines of <=80 chars are absolutely fine and should 
> > not get -1 treatment. I propose to raise the limit for the guideline on 
> > wiki accordingly.
> >
> > Comments?
> >
> > [1]: https://review.openstack.org/#/c/224728/6//COMMIT_MSG
> > [2]: https://review.openstack.org/#/c/227319/2//COMMIT_MSG
> > [3]: 
> > https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure
> > [4]: https://review.openstack.org/#/c/142585/
> > [5]: 
> > http://lists.openstack.org/pipermail/openstack-dev/2014-December/thread.html#52519
> >
> > Ihar
>
> Thanks everyone for replies.
>
> Now I realize WHY we do it with 72 chars and not 80 chars (git log output). 
> :) I updated the wiki page with how to configure Vim to enforce the rule. I 
> also removed the notion of gating on commit messages because we have them 
> removed since recently.
>
> Ihar
>
>
> __
> 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


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

2015-10-01 Thread Gary Kotton
+1

On 10/1/15, 10:56 AM, "Ghe Rivero"  wrote:

>If anyone disagrees with the commit format, please, go ahead and fix it
>(It's
>really easy using the gerrit web) For such cosmetic changes (and others
>similars), we should not wait for the author to do it. Sometimes, for a
>stupid
>comma, and with all the TZ, a change can need more than a day to be fixed
>and
>approved.
>
>Ghe Rivero
>
>Quoting Ihar Hrachyshka (2015-09-29 18:05:37)
>> > On 25 Sep 2015, at 16:44, Ihar Hrachyshka  wrote:
>> >
>> > Hi all,
>> >
>> > releases are approaching, so it¹s the right time to start some bike
>>shedding on the mailing list.
>> >
>> > Recently I got pointed out several times [1][2] that I violate our
>>commit message requirement [3] for the message lines that says:
>>"Subsequent lines should be wrapped at 72 characters.²
>> >
>> > I agree that very long commit message lines can be bad, f.e. if they
>>are 200+ chars. But <= 79 chars?.. Don¹t think so. Especially since we
>>have 79 chars limit for the code.
>> >
>> > We had a check for the line lengths in openstack-dev/hacking before
>>but it was killed [4] as per openstack-dev@ discussion [5].
>> >
>> > I believe commit message lines of <=80 chars are absolutely fine and
>>should not get -1 treatment. I propose to raise the limit for the
>>guideline on wiki accordingly.
>> >
>> > Comments?
>> >
>> > [1]: https://review.openstack.org/#/c/224728/6//COMMIT_MSG
>> > [2]: https://review.openstack.org/#/c/227319/2//COMMIT_MSG
>> > [3]: 
>>https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_m
>>essage_structure
>> > [4]: https://review.openstack.org/#/c/142585/
>> > [5]: 
>>http://lists.openstack.org/pipermail/openstack-dev/2014-December/thread.h
>>tml#52519
>> >
>> > Ihar
>>
>> Thanks everyone for replies.
>>
>> Now I realize WHY we do it with 72 chars and not 80 chars (git log
>>output). :) I updated the wiki page with how to configure Vim to enforce
>>the rule. I also removed the notion of gating on commit messages because
>>we have them removed since recently.
>>
>> Ihar
>>
>>
>> 
>>_
>>_
>> 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


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

2015-10-01 Thread Fox, Kevin M
+1.

From: Ghe Rivero [ghe.riv...@gmail.com]
Sent: Thursday, October 01, 2015 12:56 AM
To: OpenStack Development Mailing List (not for usage questions)
Subject: Re: [openstack-dev] [all] -1 due to line length violation in commit
messages

If anyone disagrees with the commit format, please, go ahead and fix it (It's
really easy using the gerrit web) For such cosmetic changes (and others
similars), we should not wait for the author to do it. Sometimes, for a stupid
comma, and with all the TZ, a change can need more than a day to be fixed and
approved.

Ghe Rivero

Quoting Ihar Hrachyshka (2015-09-29 18:05:37)
> > On 25 Sep 2015, at 16:44, Ihar Hrachyshka <ihrac...@redhat.com> wrote:
> >
> > Hi all,
> >
> > releases are approaching, so it’s the right time to start some bike 
> > shedding on the mailing list.
> >
> > Recently I got pointed out several times [1][2] that I violate our commit 
> > message requirement [3] for the message lines that says: "Subsequent lines 
> > should be wrapped at 72 characters.”
> >
> > I agree that very long commit message lines can be bad, f.e. if they are 
> > 200+ chars. But <= 79 chars?.. Don’t think so. Especially since we have 79 
> > chars limit for the code.
> >
> > We had a check for the line lengths in openstack-dev/hacking before but it 
> > was killed [4] as per openstack-dev@ discussion [5].
> >
> > I believe commit message lines of <=80 chars are absolutely fine and should 
> > not get -1 treatment. I propose to raise the limit for the guideline on 
> > wiki accordingly.
> >
> > Comments?
> >
> > [1]: https://review.openstack.org/#/c/224728/6//COMMIT_MSG
> > [2]: https://review.openstack.org/#/c/227319/2//COMMIT_MSG
> > [3]: 
> > https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure
> > [4]: https://review.openstack.org/#/c/142585/
> > [5]: 
> > http://lists.openstack.org/pipermail/openstack-dev/2014-December/thread.html#52519
> >
> > Ihar
>
> Thanks everyone for replies.
>
> Now I realize WHY we do it with 72 chars and not 80 chars (git log output). 
> :) I updated the wiki page with how to configure Vim to enforce the rule. I 
> also removed the notion of gating on commit messages because we have them 
> removed since recently.
>
> Ihar
>
>
> __
> 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


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

2015-09-30 Thread Thomas Goirand
On 09/25/2015 05:00 PM, Ryan Brown wrote:
> I believe the 72 limit is derived from 80-8 (terminal width - tab width)

If I'm not mistaking, 72 is because of the email format limitation.

Thomas


__
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


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

2015-09-30 Thread Mike Spreitzer
> From: Gorka Eguileor <gegui...@redhat.com>
> To: "OpenStack Development Mailing List (not for usage questions)" 
> <openstack-dev@lists.openstack.org>
> Date: 09/29/2015 07:34 AM
> Subject: Re: [openstack-dev] [all] -1 due to line length violation 
> in commit messages
...
> Since we are not all native speakers expecting everyone to realize that
> difference - which is completely right - may be a little optimistic,
> moreover considering that parts of those guidelines may even be written
> by non natives.
> 
> Let's say I interpret all "should" instances in that guideline as rules
> that don't need to be strictly enforced, I see that the Change-Id
> "should not be changed when rebasing" - this one would certainly be fun
> to watch if we didn't follow it - the blueprint "should give the name of
> a Launchpad blueprint" - I don't know any core that would not -1 a patch
> if he notices the BP reference missing - and machine targeted metadata
> "should all be grouped together at the end of the commit message" - this
> one everyone follows instinctively, so no problem.
> 
> And if we look at the i18n guidelines, almost everything is using
> should, but on reviews these are treated as strict *must* because of the
> implications.
> 
> Anyway, it's a matter of opinion and afaik in Cinder we don't even have
> a real problem with downvoting for the commit message length, I don't
> see more than 1 every couple of months or so.

Other communities have solved this by explicit reference to a standard 
defining terms like "must" and "should".

Regards,
Mike


__
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


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

2015-09-30 Thread Zane Bitter

On 29/09/15 12:05, Ihar Hrachyshka wrote:

On 25 Sep 2015, at 16:44, Ihar Hrachyshka  wrote:

Hi all,

releases are approaching, so it’s the right time to start some bike shedding on 
the mailing list.

Recently I got pointed out several times [1][2] that I violate our commit message 
requirement [3] for the message lines that says: "Subsequent lines should be 
wrapped at 72 characters.”

I agree that very long commit message lines can be bad, f.e. if they are 200+ 
chars. But <= 79 chars?.. Don’t think so. Especially since we have 79 chars 
limit for the code.

We had a check for the line lengths in openstack-dev/hacking before but it was 
killed [4] as per openstack-dev@ discussion [5].

I believe commit message lines of <=80 chars are absolutely fine and should not 
get -1 treatment. I propose to raise the limit for the guideline on wiki 
accordingly.

Comments?

[1]: https://review.openstack.org/#/c/224728/6//COMMIT_MSG
[2]: https://review.openstack.org/#/c/227319/2//COMMIT_MSG
[3]: 
https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure
[4]: https://review.openstack.org/#/c/142585/
[5]: 
http://lists.openstack.org/pipermail/openstack-dev/2014-December/thread.html#52519

Ihar


Thanks everyone for replies.

Now I realize WHY we do it with 72 chars and not 80 chars (git log output). :) 
I updated the wiki page with how to configure Vim to enforce the rule. I also 
removed the notion of gating on commit messages because we have them removed 
since recently.


Thanks Ihar! FWIW, vim has had built-in support for setting that width 
since at least 7.2, and I suspect long before (for me it's in 
/usr/share/vim/vim74/ftplugin/gitcommit.vim). AFAIK the only thing you 
need in your .vimrc to take advantage is:


if has("autocmd")
  filetype plugin indent on
endif " has("autocmd")

This is included in the example vimrc file that ships with vim, so I 
think better advice for 99% of people would be to just install the 
example vimrc file if they don't already have a ~/.vimrc. (There are 
*lots* of other benefits too.) I've updated the wiki to reflect that, I 
hope you don't mind :)


It'd be great if anyone who didn't have it set up already could try this 
though, since it's been many, many years since it has not worked 
automagically for me ;)


cheers,
Zane.

__
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


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

2015-09-29 Thread Gorka Eguileor
On 28/09, Clint Byrum wrote:
> Excerpts from Kevin Benton's message of 2015-09-28 14:29:14 -0700:
> > I think a blanket statement about what people's motivations are is not
> > fair. We've seen in this thread that some people want to enforce the limit
> > of 72 chars and it's not about padding their stats.
> > 
> > The issue here is that we have a guideline with a very specific number. If
> > we don't care to enforce it, why do we even bother? "Please do this, unless
> > you don't feel like it", is going to be hard for many people to review in a
> > way that pleases everyone.
> > 
> 
> Please do read said guidelines. "Must" would be used if it were to be
> "enforced". It "should" be formatted that way.

Since we are not all native speakers expecting everyone to realize that
difference - which is completely right - may be a little optimistic,
moreover considering that parts of those guidelines may even be written
by non natives.

Let's say I interpret all "should" instances in that guideline as rules
that don't need to be strictly enforced, I see that the Change-Id
"should not be changed when rebasing" - this one would certainly be fun
to watch if we didn't follow it - the blueprint "should give the name of
a Launchpad blueprint" - I don't know any core that would not -1 a patch
if he notices the BP reference missing - and machine targeted metadata
"should all be grouped together at the end of the commit message" - this
one everyone follows instinctively, so no problem.

And if we look at the i18n guidelines, almost everything is using
should, but on reviews these are treated as strict *must* because of the
implications.

Anyway, it's a matter of opinion and afaik in Cinder we don't even have
a real problem with downvoting for the commit message length, I don't
see more than 1 every couple of months or so.

Cheers,
Gorka.

> 
> > On Mon, Sep 28, 2015 at 11:00 PM, Assaf Muller  wrote:
> > 
> > >
> > >
> > > On Mon, Sep 28, 2015 at 12:40 PM, Zane Bitter  wrote:
> > >
> > >> On 28/09/15 05:47, 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.
> > 
> > >>>
> > >> +1
> > >>
> > >> 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.
> > 
> > >>>
> > >> +1
> > >>
> > >> 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.
> > >>>
> > >>
> > >> Apparently you're unaware of the definition of the word 'guideline'. It's
> > >> a guide. If it were a hard-and-fast rule then we would have a bot 
> > >> enforcing
> > >> it already.
> > >>
> > >> Is there anything quite so frightening as a large group of people blindly
> > >> enforcing rules with total indifference to any sense of overarching 
> > >> purpose?
> > >>
> > >> A reminder that the reason for this guideline is to ensure that none of
> > >> the broad variety of tools that are available in the Git ecosystem
> > >> effectively become unusable with the OpenStack repos due to wildly
> > >> inconsistent formatting. And of course, even that goal has to be balanced
> > >> against our other goals, such as building a healthy community and
> > >> occasionally shipping some software.
> > >>
> > >> There are plenty of ways to achieve that goal other than blanket drive-by
> > >> -1's for trivial inconsistencies in the formatting of individual commit
> > >> messages.
> > >
> > >
> > > The actual issue is that we as a community (Speaking of the Neutron
> > > community at least) are stat-crazed. We have a fair number of contributors
> > > that -1 for trivial issues to retain their precious stats with alarming
> > > zeal. That is the real issue. All of these commit message issues,
> > > translation mishaps,
> > > comment typos etc are excuses for people to boost their stats without
> > > contributing their time or energy in to the project. I am beyond bitter
> > > about this
> > > issue at 

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

2015-09-29 Thread Ihar Hrachyshka
> On 25 Sep 2015, at 16:44, Ihar Hrachyshka  wrote:
> 
> Hi all,
> 
> releases are approaching, so it’s the right time to start some bike shedding 
> on the mailing list.
> 
> Recently I got pointed out several times [1][2] that I violate our commit 
> message requirement [3] for the message lines that says: "Subsequent lines 
> should be wrapped at 72 characters.”
> 
> I agree that very long commit message lines can be bad, f.e. if they are 200+ 
> chars. But <= 79 chars?.. Don’t think so. Especially since we have 79 chars 
> limit for the code.
> 
> We had a check for the line lengths in openstack-dev/hacking before but it 
> was killed [4] as per openstack-dev@ discussion [5].
> 
> I believe commit message lines of <=80 chars are absolutely fine and should 
> not get -1 treatment. I propose to raise the limit for the guideline on wiki 
> accordingly.
> 
> Comments?
> 
> [1]: https://review.openstack.org/#/c/224728/6//COMMIT_MSG
> [2]: https://review.openstack.org/#/c/227319/2//COMMIT_MSG
> [3]: 
> https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure
> [4]: https://review.openstack.org/#/c/142585/
> [5]: 
> http://lists.openstack.org/pipermail/openstack-dev/2014-December/thread.html#52519
> 
> Ihar

Thanks everyone for replies.

Now I realize WHY we do it with 72 chars and not 80 chars (git log output). :) 
I updated the wiki page with how to configure Vim to enforce the rule. I also 
removed the notion of gating on commit messages because we have them removed 
since recently.

Ihar


signature.asc
Description: Message signed with OpenPGP using GPGMail
__
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


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

2015-09-28 Thread Zane Bitter

On 28/09/15 05:47, 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.


+1


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.


+1


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.


Apparently you're unaware of the definition of the word 'guideline'. 
It's a guide. If it were a hard-and-fast rule then we would have a bot 
enforcing it already.


Is there anything quite so frightening as a large group of people 
blindly enforcing rules with total indifference to any sense of 
overarching purpose?


A reminder that the reason for this guideline is to ensure that none of 
the broad variety of tools that are available in the Git ecosystem 
effectively become unusable with the OpenStack repos due to wildly 
inconsistent formatting. And of course, even that goal has to be 
balanced against our other goals, such as building a healthy community 
and occasionally shipping some software.


There are plenty of ways to achieve that goal other than blanket 
drive-by -1's for trivial inconsistencies in the formatting of 
individual commit messages. A polite comment and a link to the 
guidelines is a great way to educate new contributors. For core 
reviewers especially, a comment like that and a +1 review will *almost 
always* get you the change you want in double-quick time. (Any 
contributor who knows they are 30s work away from a +2 is going to be 
highly motivated.)



Typos are a completely different matter and they should not be grouped
together with guideline infringements.


"Violations"? "Infringements"? It's line wrapping, not a felony case.


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.


Of course it is.

If we're not here to use our brains, why are we here? Serious question. 
Feel free to use any definition of 'here'.



Cheers,
Gorka.




On Sep 26, 2015, at 21:19, Ian Wells  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.


I would welcome a confirmation step - but *not* an outright rejection - 
that runs *locally* in git-review before the change is pushed. Right 
now, gerrit gives you a warning after the review is pushed, at which 
point it is too late.



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.


Yes, 72 columns is the correct guideline IMHO. It's used virtually 
throughout the Git ecosystem now. Back in the early days of Git it 
wasn't at all clear - should you have no line breaks at all and let each 
tool do its own soft line wrapping? If not, where should you wrap? Now 
there's a clear consensus that you hard wrap at 72. Vi wraps git commit 
messages at 72 by default.


The output of "git log" indents commit messages by four spaces, so 
anything longer than 76 gets ugly, hard-to-read line-wrapping. I've also 
noticed that Launchpad (or at least the bot that posts commit messages 
to Launchpad when patches merge) does a hard wrap at 72 characters.


A much better idea than modifying the guideline would be to put 
documentation on the wiki about how to set up your editor so that this 
is never an issue. You shouldn't even have to 

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

2015-09-28 Thread Clint Byrum
Excerpts from Kevin Benton's message of 2015-09-28 14:29:14 -0700:
> I think a blanket statement about what people's motivations are is not
> fair. We've seen in this thread that some people want to enforce the limit
> of 72 chars and it's not about padding their stats.
> 
> The issue here is that we have a guideline with a very specific number. If
> we don't care to enforce it, why do we even bother? "Please do this, unless
> you don't feel like it", is going to be hard for many people to review in a
> way that pleases everyone.
> 

Please do read said guidelines. "Must" would be used if it were to be
"enforced". It "should" be formatted that way.

> On Mon, Sep 28, 2015 at 11:00 PM, Assaf Muller  wrote:
> 
> >
> >
> > On Mon, Sep 28, 2015 at 12:40 PM, Zane Bitter  wrote:
> >
> >> On 28/09/15 05:47, 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.
> 
> >>>
> >> +1
> >>
> >> 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.
> 
> >>>
> >> +1
> >>
> >> 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.
> >>>
> >>
> >> Apparently you're unaware of the definition of the word 'guideline'. It's
> >> a guide. If it were a hard-and-fast rule then we would have a bot enforcing
> >> it already.
> >>
> >> Is there anything quite so frightening as a large group of people blindly
> >> enforcing rules with total indifference to any sense of overarching 
> >> purpose?
> >>
> >> A reminder that the reason for this guideline is to ensure that none of
> >> the broad variety of tools that are available in the Git ecosystem
> >> effectively become unusable with the OpenStack repos due to wildly
> >> inconsistent formatting. And of course, even that goal has to be balanced
> >> against our other goals, such as building a healthy community and
> >> occasionally shipping some software.
> >>
> >> There are plenty of ways to achieve that goal other than blanket drive-by
> >> -1's for trivial inconsistencies in the formatting of individual commit
> >> messages.
> >
> >
> > The actual issue is that we as a community (Speaking of the Neutron
> > community at least) are stat-crazed. We have a fair number of contributors
> > that -1 for trivial issues to retain their precious stats with alarming
> > zeal. That is the real issue. All of these commit message issues,
> > translation mishaps,
> > comment typos etc are excuses for people to boost their stats without
> > contributing their time or energy in to the project. I am beyond bitter
> > about this
> > issue at this point.
> >
> > I'll say what I've always said about this issue: The review process is
> > about collaboration. I imagine that the author is sitting next to me, and
> > we're going
> > through the patch together for the purpose of improving it. Review
> > comments should be motivated by a thirst to improve the proposed code in a
> > real way,
> > not by your want or need to improve your stats on stackalytics. The latter
> > is an enormous waste of your time.
> >
> >
> >> A polite comment and a link to the guidelines is a great way to educate
> >> new contributors. For core reviewers especially, a comment like that and a
> >> +1 review will *almost always* get you the change you want in double-quick
> >> time. (Any contributor who knows they are 30s work away from a +2 is going
> >> to be highly motivated.)
> >>
> >> Typos are a completely different matter and they should not be grouped
> >>> together with guideline infringements.
> >>>
> >>
> >> "Violations"? "Infringements"? It's line wrapping, not a felony case.
> >>
> >> 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 

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

2015-09-28 Thread Kevin Benton
I think a blanket statement about what people's motivations are is not
fair. We've seen in this thread that some people want to enforce the limit
of 72 chars and it's not about padding their stats.

The issue here is that we have a guideline with a very specific number. If
we don't care to enforce it, why do we even bother? "Please do this, unless
you don't feel like it", is going to be hard for many people to review in a
way that pleases everyone.

On Mon, Sep 28, 2015 at 11:00 PM, Assaf Muller  wrote:

>
>
> On Mon, Sep 28, 2015 at 12:40 PM, Zane Bitter  wrote:
>
>> On 28/09/15 05:47, 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.

>>>
>> +1
>>
>> 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.

>>>
>> +1
>>
>> 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.
>>>
>>
>> Apparently you're unaware of the definition of the word 'guideline'. It's
>> a guide. If it were a hard-and-fast rule then we would have a bot enforcing
>> it already.
>>
>> Is there anything quite so frightening as a large group of people blindly
>> enforcing rules with total indifference to any sense of overarching purpose?
>>
>> A reminder that the reason for this guideline is to ensure that none of
>> the broad variety of tools that are available in the Git ecosystem
>> effectively become unusable with the OpenStack repos due to wildly
>> inconsistent formatting. And of course, even that goal has to be balanced
>> against our other goals, such as building a healthy community and
>> occasionally shipping some software.
>>
>> There are plenty of ways to achieve that goal other than blanket drive-by
>> -1's for trivial inconsistencies in the formatting of individual commit
>> messages.
>
>
> The actual issue is that we as a community (Speaking of the Neutron
> community at least) are stat-crazed. We have a fair number of contributors
> that -1 for trivial issues to retain their precious stats with alarming
> zeal. That is the real issue. All of these commit message issues,
> translation mishaps,
> comment typos etc are excuses for people to boost their stats without
> contributing their time or energy in to the project. I am beyond bitter
> about this
> issue at this point.
>
> I'll say what I've always said about this issue: The review process is
> about collaboration. I imagine that the author is sitting next to me, and
> we're going
> through the patch together for the purpose of improving it. Review
> comments should be motivated by a thirst to improve the proposed code in a
> real way,
> not by your want or need to improve your stats on stackalytics. The latter
> is an enormous waste of your time.
>
>
>> A polite comment and a link to the guidelines is a great way to educate
>> new contributors. For core reviewers especially, a comment like that and a
>> +1 review will *almost always* get you the change you want in double-quick
>> time. (Any contributor who knows they are 30s work away from a +2 is going
>> to be highly motivated.)
>>
>> Typos are a completely different matter and they should not be grouped
>>> together with guideline infringements.
>>>
>>
>> "Violations"? "Infringements"? It's line wrapping, not a felony case.
>>
>> 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.
>>>
>>
>> Of course it is.
>>
>> If we're not here to use our brains, why are we here? Serious question.
>> Feel free to use 

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

2015-09-28 Thread Assaf Muller
On Mon, Sep 28, 2015 at 12:40 PM, Zane Bitter  wrote:

> On 28/09/15 05:47, 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.
>>>
>>
> +1
>
> 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.
>>>
>>
> +1
>
> 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.
>>
>
> Apparently you're unaware of the definition of the word 'guideline'. It's
> a guide. If it were a hard-and-fast rule then we would have a bot enforcing
> it already.
>
> Is there anything quite so frightening as a large group of people blindly
> enforcing rules with total indifference to any sense of overarching purpose?
>
> A reminder that the reason for this guideline is to ensure that none of
> the broad variety of tools that are available in the Git ecosystem
> effectively become unusable with the OpenStack repos due to wildly
> inconsistent formatting. And of course, even that goal has to be balanced
> against our other goals, such as building a healthy community and
> occasionally shipping some software.
>
> There are plenty of ways to achieve that goal other than blanket drive-by
> -1's for trivial inconsistencies in the formatting of individual commit
> messages.


The actual issue is that we as a community (Speaking of the Neutron
community at least) are stat-crazed. We have a fair number of contributors
that -1 for trivial issues to retain their precious stats with alarming
zeal. That is the real issue. All of these commit message issues,
translation mishaps,
comment typos etc are excuses for people to boost their stats without
contributing their time or energy in to the project. I am beyond bitter
about this
issue at this point.

I'll say what I've always said about this issue: The review process is
about collaboration. I imagine that the author is sitting next to me, and
we're going
through the patch together for the purpose of improving it. Review comments
should be motivated by a thirst to improve the proposed code in a real way,
not by your want or need to improve your stats on stackalytics. The latter
is an enormous waste of your time.


> A polite comment and a link to the guidelines is a great way to educate
> new contributors. For core reviewers especially, a comment like that and a
> +1 review will *almost always* get you the change you want in double-quick
> time. (Any contributor who knows they are 30s work away from a +2 is going
> to be highly motivated.)
>
> Typos are a completely different matter and they should not be grouped
>> together with guideline infringements.
>>
>
> "Violations"? "Infringements"? It's line wrapping, not a felony case.
>
> 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.
>>
>
> Of course it is.
>
> If we're not here to use our brains, why are we here? Serious question.
> Feel free to use any definition of 'here'.
>
> Cheers,
>> Gorka.
>>
>>
>>
>> On Sep 26, 2015, at 21:19, Ian Wells  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.

>>>
> I would welcome a confirmation step - but *not* an outright rejection -
> 

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

2015-09-28 Thread Clint Byrum
Excerpts from Morgan Fainberg's message of 2015-09-26 23:36:09 -0700:
> 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. 
> 

Agreed with all of your sentiments.

Please anyone -1'ing for this, read this:

https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure


"The first line should be limited to 50 characters and should not end
with a period (commit messages over 72 characters will be rejected by
the gate)."

"Subsequent lines should be wrapped at 72 characters."

Notice, the word "should" is used, not "must".

So _DO NOT_ -1 for this. A "should" is a guideline, and a note like
"hey could you wrap at 72 chars if you push again? We like to keep them
formatted that way, see [link] for more info. Thanks! #notAMinusOne"

Please can we not spend another minute on this? Thanks!

__
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


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

2015-09-28 Thread Kyle Mestery
On Mon, Sep 28, 2015 at 4:00 PM, Assaf Muller  wrote:

>
>
> On Mon, Sep 28, 2015 at 12:40 PM, Zane Bitter  wrote:
>
>> On 28/09/15 05:47, 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.

>>>
>> +1
>>
>> 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.

>>>
>> +1
>>
>> 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.
>>>
>>
>> Apparently you're unaware of the definition of the word 'guideline'. It's
>> a guide. If it were a hard-and-fast rule then we would have a bot enforcing
>> it already.
>>
>> Is there anything quite so frightening as a large group of people blindly
>> enforcing rules with total indifference to any sense of overarching purpose?
>>
>> A reminder that the reason for this guideline is to ensure that none of
>> the broad variety of tools that are available in the Git ecosystem
>> effectively become unusable with the OpenStack repos due to wildly
>> inconsistent formatting. And of course, even that goal has to be balanced
>> against our other goals, such as building a healthy community and
>> occasionally shipping some software.
>>
>> There are plenty of ways to achieve that goal other than blanket drive-by
>> -1's for trivial inconsistencies in the formatting of individual commit
>> messages.
>
>
> The actual issue is that we as a community (Speaking of the Neutron
> community at least) are stat-crazed. We have a fair number of contributors
> that -1 for trivial issues to retain their precious stats with alarming
> zeal. That is the real issue. All of these commit message issues,
> translation mishaps,
> comment typos etc are excuses for people to boost their stats without
> contributing their time or energy in to the project. I am beyond bitter
> about this
> issue at this point.
>
>
I should note that as the previous PTL, for the most part I viewed stats as
garbage. Keep in mind I nominated two new core reviewers whose stats were
lowe but who are incredibly important members of our community [1]. I did
this because they are the type of people to be core reviewers, and we had a
long conversation on this. So, I agree with you, this stats thing is awful.
And Stackalytics hasn't helped it, but made it much worse.

[1]
http://lists.openstack.org/pipermail/openstack-dev/2015-August/071869.html


> I'll say what I've always said about this issue: The review process is
> about collaboration. I imagine that the author is sitting next to me, and
> we're going
> through the patch together for the purpose of improving it. Review
> comments should be motivated by a thirst to improve the proposed code in a
> real way,
> not by your want or need to improve your stats on stackalytics. The latter
> is an enormous waste of your time.
>
>
>> A polite comment and a link to the guidelines is a great way to educate
>> new contributors. For core reviewers especially, a comment like that and a
>> +1 review will *almost always* get you the change you want in double-quick
>> time. (Any contributor who knows they are 30s work away from a +2 is going
>> to be highly motivated.)
>>
>> Typos are a completely different matter and they should not be grouped
>>> together with guideline infringements.
>>>
>>
>> "Violations"? "Infringements"? It's line wrapping, not a felony case.
>>
>> 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.
>>>
>>
>> Of course it is.
>>
>> If we're not here to use our 

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

2015-09-28 Thread Gorka Eguileor
On 28/09, Andreas Jaeger wrote:
> On 2015-09-28 11:47, 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.
> > [...]
> 
> You could also tell the committer about the length so that s/he learns for
> the next time. Giving a -1 just for a few lines that are 80 chars long is
> over the top IMHO,
> 
> Andreas
> -- 

I tell the committer of this guideline, just like it was told to me on
my first commits; and I agree that it sucks to give or receive a -1 for
this, but let me put it this way, how many times will you be
getting/giving a -1 to the same person for this?

If it's a first time committer you'll probably say it once, they'll
learn it, fix it and them we have all our commits conforming to our
guidelines, not such a big deal (although I agree with Miguel Angel that
this should be automated) and if it's not a first time committer he
should have known better and he deserves the -1 for not paying attention
and/or not having his dev env properly setup.


>  Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
>   SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
>GF: Felix Imendörffer, Jane Smithard, Graham Norton,
>HRB 21284 (AG Nürnberg)
> GPG fingerprint = 93A3 365E CE47 B889 DF7F FED1 389A 563C C272 A126
> 
> 
> __
> 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


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

2015-09-28 Thread Andreas Jaeger

On 2015-09-28 11:47, 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.

> [...]

You could also tell the committer about the length so that s/he learns 
for the next time. Giving a -1 just for a few lines that are 80 chars 
long is over the top IMHO,


Andreas
--
 Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
  SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GF: Felix Imendörffer, Jane Smithard, Graham Norton,
   HRB 21284 (AG Nürnberg)
GPG fingerprint = 93A3 365E CE47 B889 DF7F FED1 389A 563C C272 A126


__
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


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

2015-09-28 Thread Miguel Angel Ajo

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  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  wrote:
On 2015-09-25 16:15:15 + (+), 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  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



__
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


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

2015-09-28 Thread Victor Stinner

Le 25/09/2015 17:00, Julien Danjou a écrit :

If we wanted to enforce that, we would just have to write a bot setting
-1 automatically. I'm getting tired of seeing people doing bots' jobs in
Gerrit.


It may also help to enhance "git review -s" to install a local 
post-commit hook to warn if the commit message doesn't respect OpenStack 
coding style.


(Like the evil "final point", haha. I don't recall if it was mandatory 
or illegal.)


Victor

__
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


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

2015-09-28 Thread Assaf Muller
On Mon, Sep 28, 2015 at 5:29 PM, Kevin Benton  wrote:

> I think a blanket statement about what people's motivations are is not
> fair. We've seen in this thread that some people want to enforce the limit
> of 72 chars and it's not about padding their stats.
>

I took this golden opportunity to kidnap the thread and invoke a general
rant, it's not specific to the 72 characters git commit title discussion.


>
> The issue here is that we have a guideline with a very specific number. If
> we don't care to enforce it, why do we even bother? "Please do this, unless
> you don't feel like it", is going to be hard for many people to review in a
> way that pleases everyone.
>
> On Mon, Sep 28, 2015 at 11:00 PM, Assaf Muller  wrote:
>
>>
>>
>> On Mon, Sep 28, 2015 at 12:40 PM, Zane Bitter  wrote:
>>
>>> On 28/09/15 05:47, 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.
>

>>> +1
>>>
>>> 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.
>

>>> +1
>>>
>>> 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.

>>>
>>> Apparently you're unaware of the definition of the word 'guideline'.
>>> It's a guide. If it were a hard-and-fast rule then we would have a bot
>>> enforcing it already.
>>>
>>> Is there anything quite so frightening as a large group of people
>>> blindly enforcing rules with total indifference to any sense of overarching
>>> purpose?
>>>
>>> A reminder that the reason for this guideline is to ensure that none of
>>> the broad variety of tools that are available in the Git ecosystem
>>> effectively become unusable with the OpenStack repos due to wildly
>>> inconsistent formatting. And of course, even that goal has to be balanced
>>> against our other goals, such as building a healthy community and
>>> occasionally shipping some software.
>>>
>>> There are plenty of ways to achieve that goal other than blanket
>>> drive-by -1's for trivial inconsistencies in the formatting of individual
>>> commit messages.
>>
>>
>> The actual issue is that we as a community (Speaking of the Neutron
>> community at least) are stat-crazed. We have a fair number of contributors
>> that -1 for trivial issues to retain their precious stats with alarming
>> zeal. That is the real issue. All of these commit message issues,
>> translation mishaps,
>> comment typos etc are excuses for people to boost their stats without
>> contributing their time or energy in to the project. I am beyond bitter
>> about this
>> issue at this point.
>>
>> I'll say what I've always said about this issue: The review process is
>> about collaboration. I imagine that the author is sitting next to me, and
>> we're going
>> through the patch together for the purpose of improving it. Review
>> comments should be motivated by a thirst to improve the proposed code in a
>> real way,
>> not by your want or need to improve your stats on stackalytics. The
>> latter is an enormous waste of your time.
>>
>>
>>> A polite comment and a link to the guidelines is a great way to educate
>>> new contributors. For core reviewers especially, a comment like that and a
>>> +1 review will *almost always* get you the change you want in double-quick
>>> time. (Any contributor who knows they are 30s work away from a +2 is going
>>> to be highly motivated.)
>>>
>>> Typos are a completely different matter and they should not be grouped
 together with guideline infringements.

>>>
>>> "Violations"? "Infringements"? It's line wrapping, not a felony case.
>>>
>>> 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 

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

2015-09-28 Thread Doug Wiegley

> On Sep 28, 2015, at 3:00 PM, Assaf Muller  wrote:
> 
> 
> 
> On Mon, Sep 28, 2015 at 12:40 PM, Zane Bitter  > wrote:
> On 28/09/15 05:47, 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.
> 
> +1
> 
> 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.
> 
> +1
> 
> 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.
> 
> Apparently you're unaware of the definition of the word 'guideline'. It's a 
> guide. If it were a hard-and-fast rule then we would have a bot enforcing it 
> already.
> 
> Is there anything quite so frightening as a large group of people blindly 
> enforcing rules with total indifference to any sense of overarching purpose?
> 
> A reminder that the reason for this guideline is to ensure that none of the 
> broad variety of tools that are available in the Git ecosystem effectively 
> become unusable with the OpenStack repos due to wildly inconsistent 
> formatting. And of course, even that goal has to be balanced against our 
> other goals, such as building a healthy community and occasionally shipping 
> some software.
> 
> There are plenty of ways to achieve that goal other than blanket drive-by 
> -1's for trivial inconsistencies in the formatting of individual commit 
> messages.
> 
> The actual issue is that we as a community (Speaking of the Neutron community 
> at least) are stat-crazed. We have a fair number of contributors
> that -1 for trivial issues to retain their precious stats with alarming zeal. 
> That is the real issue. All of these commit message issues, translation 
> mishaps,
> comment typos etc are excuses for people to boost their stats without 
> contributing their time or energy in to the project. I am beyond bitter about 
> this
> issue at this point.
> 
> I'll say what I've always said about this issue: The review process is about 
> collaboration. I imagine that the author is sitting next to me, and we're 
> going
> through the patch together for the purpose of improving it. Review comments 
> should be motivated by a thirst to improve the proposed code in a real way,
> not by your want or need to improve your stats on stackalytics. The latter is 
> an enormous waste of your time.

This is kind of a thread-jack, but to respond to your concern, I think the 
infra team has a nice writeup on how to review that addresses your concern: 
http://docs.openstack.org/infra/system-config/project.html#review-criteria 


I’ve certainly seen plenty of neutron reviewers that I’d prefer to see 
following the above link (myself included, on occasion.)

Thanks,
doug



>  
> A polite comment and a link to the guidelines is a great way to educate new 
> contributors. For core reviewers especially, a comment like that and a +1 
> review will *almost always* get you the change you want in double-quick time. 
> (Any contributor who knows they are 30s work away from a +2 is going to be 
> highly motivated.)
> 
> Typos are a completely different matter and they should not be grouped
> together with guideline infringements.
> 
> "Violations"? "Infringements"? It's line wrapping, not a felony case.
> 
> 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.
> 
> Of course it is.
> 
> If we're not here to use our brains, why are we here? Serious question. Feel 
> free to use any definition of 'here'.
> 
> Cheers,
> Gorka.
> 
> 
> 
> On Sep 

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

2015-09-28 Thread Jeremy Stanley
On 2015-09-28 16:27:19 -0500 (-0500), Kyle Mestery wrote:
[...]
> I should note that as the previous PTL, for the most part I viewed
> stats as garbage. Keep in mind I nominated two new core reviewers
> whose stats were lowe but who are incredibly important members of
> our community [1]. I did this because they are the type of people
> to be core reviewers, and we had a long conversation on this. So,
> I agree with you, this stats thing is awful. And Stackalytics
> hasn't helped it, but made it much worse.
[...]

"Any observed statistical regularity will tend to
collapse once pressure is placed upon it for control
purposes." - Charles Goodhart, 1975

https://en.wikipedia.org/wiki/Goodhart%27s_law

-- 
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


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

2015-09-27 Thread Morgan Fainberg
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

> On Sep 26, 2015, at 21:19, Ian Wells  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  wrote:
>> On 2015-09-25 16:15:15 + (+), 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 > > 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


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

2015-09-26 Thread Ian Wells
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  wrote:

> On 2015-09-25 16:15:15 + (+), 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  > 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


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

2015-09-25 Thread Vikram Choudhary
+1 for <=80 chars. It will be uniform with the existing coding style.

On Fri, Sep 25, 2015 at 8:22 PM, Dmitry Tantsur  wrote:

> On 09/25/2015 04:44 PM, Ihar Hrachyshka wrote:
>
>> Hi all,
>>
>> releases are approaching, so it’s the right time to start some bike
>> shedding on the mailing list.
>>
>> Recently I got pointed out several times [1][2] that I violate our commit
>> message requirement [3] for the message lines that says: "Subsequent lines
>> should be wrapped at 72 characters.”
>>
>> I agree that very long commit message lines can be bad, f.e. if they are
>> 200+ chars. But <= 79 chars?.. Don’t think so. Especially since we have 79
>> chars limit for the code.
>>
>> We had a check for the line lengths in openstack-dev/hacking before but
>> it was killed [4] as per openstack-dev@ discussion [5].
>>
>> I believe commit message lines of <=80 chars are absolutely fine and
>> should not get -1 treatment. I propose to raise the limit for the guideline
>> on wiki accordingly.
>>
>
> +1, I never understood it actually. I know some folks even question 80
> chars for the code, so having 72 chars for commit messages looks a bit
> weird to me.
>
>
>> Comments?
>>
>> [1]: https://review.openstack.org/#/c/224728/6//COMMIT_MSG
>> [2]: https://review.openstack.org/#/c/227319/2//COMMIT_MSG
>> [3]:
>> https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure
>> [4]: https://review.openstack.org/#/c/142585/
>> [5]:
>> http://lists.openstack.org/pipermail/openstack-dev/2014-December/thread.html#52519
>>
>> Ihar
>>
>>
>>
>> __
>> 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


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

2015-09-25 Thread Doug Hellmann
git tools such as git log and git show indent the commit message in
their output, so you don't actually have the full 79/80 character width
to work with. That's where the 72 comes from.

Doug

Excerpts from Vikram Choudhary's message of 2015-09-25 20:25:41 +0530:
> +1 for <=80 chars. It will be uniform with the existing coding style.
> 
> On Fri, Sep 25, 2015 at 8:22 PM, Dmitry Tantsur  wrote:
> 
> > On 09/25/2015 04:44 PM, Ihar Hrachyshka wrote:
> >
> >> Hi all,
> >>
> >> releases are approaching, so it’s the right time to start some bike
> >> shedding on the mailing list.
> >>
> >> Recently I got pointed out several times [1][2] that I violate our commit
> >> message requirement [3] for the message lines that says: "Subsequent lines
> >> should be wrapped at 72 characters.”
> >>
> >> I agree that very long commit message lines can be bad, f.e. if they are
> >> 200+ chars. But <= 79 chars?.. Don’t think so. Especially since we have 79
> >> chars limit for the code.
> >>
> >> We had a check for the line lengths in openstack-dev/hacking before but
> >> it was killed [4] as per openstack-dev@ discussion [5].
> >>
> >> I believe commit message lines of <=80 chars are absolutely fine and
> >> should not get -1 treatment. I propose to raise the limit for the guideline
> >> on wiki accordingly.
> >>
> >
> > +1, I never understood it actually. I know some folks even question 80
> > chars for the code, so having 72 chars for commit messages looks a bit
> > weird to me.
> >
> >
> >> Comments?
> >>
> >> [1]: https://review.openstack.org/#/c/224728/6//COMMIT_MSG
> >> [2]: https://review.openstack.org/#/c/227319/2//COMMIT_MSG
> >> [3]:
> >> https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure
> >> [4]: https://review.openstack.org/#/c/142585/
> >> [5]:
> >> http://lists.openstack.org/pipermail/openstack-dev/2014-December/thread.html#52519
> >>
> >> Ihar
> >>
> >>
> >>
> >> __
> >> 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


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

2015-09-25 Thread Jim Rollenhagen
On Fri, Sep 25, 2015 at 04:44:59PM +0200, Ihar Hrachyshka wrote:
> Hi all,
> 
> releases are approaching, so it’s the right time to start some bike shedding 
> on the mailing list.
> 
> Recently I got pointed out several times [1][2] that I violate our commit 
> message requirement [3] for the message lines that says: "Subsequent lines 
> should be wrapped at 72 characters.”
> 
> I agree that very long commit message lines can be bad, f.e. if they are 200+ 
> chars. But <= 79 chars?.. Don’t think so. Especially since we have 79 chars 
> limit for the code.
> 
> We had a check for the line lengths in openstack-dev/hacking before but it 
> was killed [4] as per openstack-dev@ discussion [5].
> 
> I believe commit message lines of <=80 chars are absolutely fine and should 
> not get -1 treatment. I propose to raise the limit for the guideline on wiki 
> accordingly.
> 
> Comments?

It makes me really sad that we actually even spend time discussing
things like this. As a core reviewer, I would just totally ignore this
-1. I also ignore -1s for things like minor typos in a comment, etc.

Let's focus on building good software instead. :)

// jim


__
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


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

2015-09-25 Thread Dmitry Tantsur

On 09/25/2015 04:44 PM, Ihar Hrachyshka wrote:

Hi all,

releases are approaching, so it’s the right time to start some bike shedding on 
the mailing list.

Recently I got pointed out several times [1][2] that I violate our commit message 
requirement [3] for the message lines that says: "Subsequent lines should be 
wrapped at 72 characters.”

I agree that very long commit message lines can be bad, f.e. if they are 200+ 
chars. But <= 79 chars?.. Don’t think so. Especially since we have 79 chars 
limit for the code.

We had a check for the line lengths in openstack-dev/hacking before but it was 
killed [4] as per openstack-dev@ discussion [5].

I believe commit message lines of <=80 chars are absolutely fine and should not 
get -1 treatment. I propose to raise the limit for the guideline on wiki 
accordingly.


+1, I never understood it actually. I know some folks even question 80 
chars for the code, so having 72 chars for commit messages looks a bit 
weird to me.




Comments?

[1]: https://review.openstack.org/#/c/224728/6//COMMIT_MSG
[2]: https://review.openstack.org/#/c/227319/2//COMMIT_MSG
[3]: 
https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure
[4]: https://review.openstack.org/#/c/142585/
[5]: 
http://lists.openstack.org/pipermail/openstack-dev/2014-December/thread.html#52519

Ihar



__
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


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

2015-09-25 Thread Ryan Brown

On 09/25/2015 10:44 AM, Ihar Hrachyshka wrote:

Hi all,

releases are approaching, so it’s the right time to start some bike
shedding on the mailing list.

Recently I got pointed out several times [1][2] that I violate our
commit message requirement [3] for the message lines that says:
"Subsequent lines should be wrapped at 72 characters.”

I agree that very long commit message lines can be bad, f.e. if they
are 200+ chars. But <= 79 chars?.. Don’t think so. Especially since
we have 79 chars limit for the code.


The default "git log" display shows the commit message already indented, 
and the tab may display as 8 spaces I suppose. I believe the 72 limit is 
derived from 80-8 (terminal width - tab width)


I don't know how many folks use 80-char terminals (I use side-by-side 
110-column terms). Having some limit to prevent 200+ is reasonable, but 
I think it's pedantic to -1 a patch due to a 78-char commit message line.



We had a check for the line lengths in openstack-dev/hacking before
but it was killed [4] as per openstack-dev@ discussion [5].

I believe commit message lines of <=80 chars are absolutely fine and
should not get -1 treatment. I propose to raise the limit for the
guideline on wiki accordingly.

Comments?

[1]: https://review.openstack.org/#/c/224728/6//COMMIT_MSG [2]:
https://review.openstack.org/#/c/227319/2//COMMIT_MSG [3]:
https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure



[4]: https://review.openstack.org/#/c/142585/

[5]:
http://lists.openstack.org/pipermail/openstack-dev/2014-December/thread.html#52519

 Ihar



__



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



--
Ryan Brown / Senior Software Engineer, Openstack / Red Hat, Inc.

__
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


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

2015-09-25 Thread Jeremy Stanley
On 2015-09-25 16:44:59 +0200 (+0200), Ihar Hrachyshka wrote:
[...]
> I believe commit message lines of <=80 chars are absolutely fine
> and should not get -1 treatment. I propose to raise the limit for
> the guideline on wiki accordingly.
[...]

As one of those traditionalists who still does all his work in 80x24
terminals (well, 80x25 with a status line), I keep my commit message
titles at or under 50 characters and message contents to no more
than 68 characters to allow for cleaner indentation/quoting just
like for my MUA editor settings. After all, some (non-OpenStack)
projects take patch submissions by E-mail and so it's easier to just
follow conservative E-mail line length conventions when possible.

That said, while I appreciate when people keep their commit message
lines wrapped short enough that they render sanely on my terminal, I
make a point of not leaving negative reviews about commit message
formatting unless it's really egregious (and usually not even then).
We have plenty of real bugs to deal with, and it's not worth my time
to berate people for inconsistent commit message layout as long as
the requisite information is present--it's easier to just lead by
example and hope that others follow most of the time.

As for the underlying topic, people leaving -1 reviews about silly,
unimportant details: reviewers need to get used to the fact that
sometimes there will be a -1 on a perfectly good proposed change, so
it's fine to ignore negative votes from people who are wasting their
time on pointless trivia. Please don't set your review filters to
skip changes with a CR -1 on them; review and judge for yourself.
-- 
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


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

2015-09-25 Thread Daniel P. Berrange
On Fri, Sep 25, 2015 at 11:05:09AM -0400, Doug Hellmann wrote:
> git tools such as git log and git show indent the commit message in
> their output, so you don't actually have the full 79/80 character width
> to work with. That's where the 72 comes from.

It is also commonly done because so that when you copy commits into
email, the commit message doesn't get further line breaks inserted.
This isn't a big deal with openstack, as we don't use an email workflow
for patch review.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

__
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


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

2015-09-25 Thread Fox, Kevin M
Yeah, and worse, since I never can remember the exact number(72 I guess), I 
always just round down to 70-1 to be safe.

Its silly.

Thanks,
Kevin

From: Ihar Hrachyshka [ihrac...@redhat.com]
Sent: Friday, September 25, 2015 7:44 AM
To: OpenStack Development Mailing List (not for usage questions)
Cc: bap...@us.ibm.com
Subject: [openstack-dev] [all] -1 due to line length violation in commit
messages

Hi all,

releases are approaching, so it’s the right time to start some bike shedding on 
the mailing list.

Recently I got pointed out several times [1][2] that I violate our commit 
message requirement [3] for the message lines that says: "Subsequent lines 
should be wrapped at 72 characters.”

I agree that very long commit message lines can be bad, f.e. if they are 200+ 
chars. But <= 79 chars?.. Don’t think so. Especially since we have 79 chars 
limit for the code.

We had a check for the line lengths in openstack-dev/hacking before but it was 
killed [4] as per openstack-dev@ discussion [5].

I believe commit message lines of <=80 chars are absolutely fine and should not 
get -1 treatment. I propose to raise the limit for the guideline on wiki 
accordingly.

Comments?

[1]: https://review.openstack.org/#/c/224728/6//COMMIT_MSG
[2]: https://review.openstack.org/#/c/227319/2//COMMIT_MSG
[3]: 
https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure
[4]: https://review.openstack.org/#/c/142585/
[5]: 
http://lists.openstack.org/pipermail/openstack-dev/2014-December/thread.html#52519

Ihar

__
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


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

2015-09-25 Thread Julien Danjou
On Fri, Sep 25 2015, Ihar Hrachyshka wrote:

> I agree that very long commit message lines can be bad, f.e. if they
> are 200+ chars. But <= 79 chars?.. Don’t think so. Especially since we
> have 79 chars limit for the code.

Agreed. <= 80 chars should be a rule of thumb and applied in a smart
way. As you say, 200+ is not OK – but we're human we can judge and be
smart.

If we wanted to enforce that, we would just have to write a bot setting
-1 automatically. I'm getting tired of seeing people doing bots' jobs in
Gerrit.

-- 
Julien Danjou
// Free Software hacker
// http://julien.danjou.info


signature.asc
Description: PGP signature
__
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


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

2015-09-25 Thread Fox, Kevin M
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  length lines on merge?

Thanks,
Kevin

From: Jim Rollenhagen [j...@jimrollenhagen.com]
Sent: Friday, September 25, 2015 8:42 AM
To: OpenStack Development Mailing List (not for usage questions)
Subject: Re: [openstack-dev] [all] -1 due to line length violation in commit 
messages

On Fri, Sep 25, 2015 at 04:44:59PM +0200, Ihar Hrachyshka wrote:
> Hi all,
>
> releases are approaching, so it’s the right time to start some bike shedding 
> on the mailing list.
>
> Recently I got pointed out several times [1][2] that I violate our commit 
> message requirement [3] for the message lines that says: "Subsequent lines 
> should be wrapped at 72 characters.”
>
> I agree that very long commit message lines can be bad, f.e. if they are 200+ 
> chars. But <= 79 chars?.. Don’t think so. Especially since we have 79 chars 
> limit for the code.
>
> We had a check for the line lengths in openstack-dev/hacking before but it 
> was killed [4] as per openstack-dev@ discussion [5].
>
> I believe commit message lines of <=80 chars are absolutely fine and should 
> not get -1 treatment. I propose to raise the limit for the guideline on wiki 
> accordingly.
>
> Comments?

It makes me really sad that we actually even spend time discussing
things like this. As a core reviewer, I would just totally ignore this
-1. I also ignore -1s for things like minor typos in a comment, etc.

Let's focus on building good software instead. :)

// jim


__
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-dev] [all] -1 due to line length violation in commit messages

2015-09-25 Thread Ihar Hrachyshka
Hi all,

releases are approaching, so it’s the right time to start some bike shedding on 
the mailing list.

Recently I got pointed out several times [1][2] that I violate our commit 
message requirement [3] for the message lines that says: "Subsequent lines 
should be wrapped at 72 characters.”

I agree that very long commit message lines can be bad, f.e. if they are 200+ 
chars. But <= 79 chars?.. Don’t think so. Especially since we have 79 chars 
limit for the code.

We had a check for the line lengths in openstack-dev/hacking before but it was 
killed [4] as per openstack-dev@ discussion [5].

I believe commit message lines of <=80 chars are absolutely fine and should not 
get -1 treatment. I propose to raise the limit for the guideline on wiki 
accordingly.

Comments?

[1]: https://review.openstack.org/#/c/224728/6//COMMIT_MSG
[2]: https://review.openstack.org/#/c/227319/2//COMMIT_MSG
[3]: 
https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure
[4]: https://review.openstack.org/#/c/142585/
[5]: 
http://lists.openstack.org/pipermail/openstack-dev/2014-December/thread.html#52519

Ihar


signature.asc
Description: Message signed with OpenPGP using GPGMail
__
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


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

2015-09-25 Thread Ivan Kolodyazhny
+1 for <=80 chars. 72 characters sometimes are not enough

Regards,
Ivan Kolodyazhny

On Fri, Sep 25, 2015 at 6:05 PM, Doug Hellmann 
wrote:

> git tools such as git log and git show indent the commit message in
> their output, so you don't actually have the full 79/80 character width
> to work with. That's where the 72 comes from.
>
> Doug
>
> Excerpts from Vikram Choudhary's message of 2015-09-25 20:25:41 +0530:
> > +1 for <=80 chars. It will be uniform with the existing coding style.
> >
> > On Fri, Sep 25, 2015 at 8:22 PM, Dmitry Tantsur 
> wrote:
> >
> > > On 09/25/2015 04:44 PM, Ihar Hrachyshka wrote:
> > >
> > >> Hi all,
> > >>
> > >> releases are approaching, so it’s the right time to start some bike
> > >> shedding on the mailing list.
> > >>
> > >> Recently I got pointed out several times [1][2] that I violate our
> commit
> > >> message requirement [3] for the message lines that says: "Subsequent
> lines
> > >> should be wrapped at 72 characters.”
> > >>
> > >> I agree that very long commit message lines can be bad, f.e. if they
> are
> > >> 200+ chars. But <= 79 chars?.. Don’t think so. Especially since we
> have 79
> > >> chars limit for the code.
> > >>
> > >> We had a check for the line lengths in openstack-dev/hacking before
> but
> > >> it was killed [4] as per openstack-dev@ discussion [5].
> > >>
> > >> I believe commit message lines of <=80 chars are absolutely fine and
> > >> should not get -1 treatment. I propose to raise the limit for the
> guideline
> > >> on wiki accordingly.
> > >>
> > >
> > > +1, I never understood it actually. I know some folks even question 80
> > > chars for the code, so having 72 chars for commit messages looks a bit
> > > weird to me.
> > >
> > >
> > >> Comments?
> > >>
> > >> [1]: https://review.openstack.org/#/c/224728/6//COMMIT_MSG
> > >> [2]: https://review.openstack.org/#/c/227319/2//COMMIT_MSG
> > >> [3]:
> > >>
> https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure
> > >> [4]: https://review.openstack.org/#/c/142585/
> > >> [5]:
> > >>
> http://lists.openstack.org/pipermail/openstack-dev/2014-December/thread.html#52519
> > >>
> > >> Ihar
> > >>
> > >>
> > >>
> > >>
> __
> > >> 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


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

2015-09-25 Thread Russell Bryant
On 09/25/2015 12:15 PM, 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  here> length lines on merge?

I don't think gerrit should mess with the commit message at all.  Commit
message formatting is often very intentional.

-- 
Russell Bryant

__
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


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

2015-09-25 Thread Jeremy Stanley
On 2015-09-25 16:15:15 + (+), 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  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