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