[openstack-dev] [hacking] rules for removal
clint at fewbar.com
Tue Jun 24 20:56:04 UTC 2014
Excerpts from Mark McLoughlin's message of 2014-06-24 12:49:52 -0700:
> On Tue, 2014-06-24 at 09:51 -0700, Clint Byrum wrote:
> > Excerpts from Monty Taylor's message of 2014-06-24 06:48:06 -0700:
> > > On 06/22/2014 02:49 PM, Duncan Thomas wrote:
> > > > On 22 June 2014 14:41, Amrith Kumar <amrith at tesora.com> wrote:
> > > >> In addition to making changes to the hacking rules, why don't we mandate also
> > > >> that perceived problems in the commit message shall not be an acceptable
> > > >> reason to -1 a change.
> > > >
> > > > -1.
> > > >
> > > > There are some /really/ bad commit messages out there, and some of us
> > > > try to use the commit messages to usefully sort through the changes
> > > > (i.e. I often -1 in cinder a change only affects one driver and that
> > > > isn't clear from the summary).
> > > >
> > > > If the perceived problem is grammatical, I'm a bit more on board with
> > > > it not a reason to rev a patch, but core reviewers can +2/A over the
> > > > top of a -1 anyway...
> > >
> > > 100% agree. Spelling and grammar are rude to review on - especially
> > > since we have (and want) a LOT of non-native English speakers. It's not
> > > our job to teach people better grammar. Heck - we have people from
> > > different English backgrounds with differing disagreements on what good
> > > grammar _IS_
> > >
> > We shouldn't quibble over _anything_ grammatical in a commit message. If
> > there is a disagreement about it, the comments should be ignored. There
> > are definitely a few grammar rules that are loose and those should be
> > largely ignored.
> > However, we should correct grammar when there is a clear solution, as
> > those same people who do not speak English as their first language are
> > likely to be confused by poor grammar.
> > We're not doing it to teach grammar. We're doing it to ensure readability.
> The importance of clear English varies with context, but commit messages
> are a place where we should try hard to just let it go, particularly
> with those who do not speak English as their first language.
> Commit messages stick around forever and it's important that they are
> useful, but they will be read by a small number of people who are going
> to be in a position to spend a small amount of time getting over
> whatever dissonance is caused by a typo or imperfect grammar.
The times that one is reading git messages are often the most stressful
such as when a regression has occurred in production.
Given that, I believe it is entirely worth it to me that the commit
messages on my patches are accurate and understandable. I embrace all
feedback which leads to them being more clear. I will of course stand
back from grammar correcting and not block patches if there are many
> I think specs are pretty similar and don't warrant much additional
> grammar nitpicking. Sure, they're longer pieces of text and slightly
> more people will rely on them for information, but they're not intended
> to be complete documentation.
Disagree. I will only state this one more time as I think everyone knows
how I feel: if we are going to grow beyond the english-as-a-first-language
world we simply cannot assume that those reading specs will be native
speakers. Good spelling and grammar helps us grow. Bad spelling and
grammar holds us back.
> Where grammar is so poor that readers would be easily misled in
> important ways, then sure that should be fixed. But there comes a point
> when we're no longer working to avoid confusion and instead just being
> pendants. Taking issue with this:
> "whatever scaling mechanism Heat and we end up going with."
> because it has a "dangling preposition" is an example of going way
> beyond the point of productive pedantry IMHO :-)
I actually agree that it would not at all be a reason to block a patch.
However, there is some ambiguity in that sentence that may not be clear
to a native speaker. It is not 100% clear if we are going with Heat,
or with the scaling mechanism. That is the only reason for the dangling
preposition debate. However, there is a debate, and thus I would _never_
block a patch based on this rule. It was feedback.. just as sometimes
there is feedback in commit messages that isn't taken and doesn't lead
to a -1.
More information about the OpenStack-dev