[openstack-dev] [hacking] rules for removal

Mark McLoughlin markmc at redhat.com
Tue Jun 24 21:26:44 UTC 2014


On Tue, 2014-06-24 at 13:56 -0700, Clint Byrum wrote:
> 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
> who disagree.
> 
> > 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.

There's two sides to this coin - concern about alienating
non-english-as-a-first-language speakers who feel undervalued because
their language is nitpicked to death and concern about alienating
english-as-a-first-language speakers who struggle to understand unclear
or incorrect language.

Obviously there's a balance to be struck there and different people will
judge that differently, but I'm personally far more concerned about the
former rather than the latter case.

I expect many beyond the english-as-a-first-language world are pretty
used to dealing with imperfect language but aren't so delighted with
being constantly reminded that their use language is imperfect.

> > 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[1] 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.

I'd wager you'd seriously struggle to find anyone who would interpret
that sentence as "we are going with Heat", even if they were
non-english-as-a-first-language speakers who had never heard of
OpenStack or Heat or scaling mechanisms.

You'll find plenty who would stumble over the sentence and need to
re-read it, but we making design spec reading a completely frictionless
experience is hardly a priority.

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

Absolutely, and I try and be clear about that with e.g. "not a -1" or
"if you're rebasing anyway, perhaps fix this".

Maybe a convention for such comments would be a good thing? We often do
'nitpick' or 'femtonit', but they are often still things people are
-1ing on.

Mark.




More information about the OpenStack-dev mailing list