Re: [openstack-dev] [Ironic] patches that only address grammatical/typos

2015-02-26 Thread Sean Dague
On 02/26/2015 06:47 AM, Lucas Alvares Gomes wrote:
> Hi,
> 
> I never had a strong opinion on this but reading what Jay said makes
> sense to me. I also like Robert suggestion about having a single +2/+A
> for such small changes.

+A

> 
> Cheers,
> Lucas
> 
> On Wed, Feb 25, 2015 at 11:22 PM, Robert Collins
>  wrote:
>> On 26 February 2015 at 05:26, Ruby Loo  wrote:
>>> Hi,
>>>
>>> I was wondering what people thought about patches that only fix grammatical
>>> issues or misspellings in comments in our code.
>>>
>>> I can't believe I'm sending out this email, but as a group, I'd like it if
>>> we had  a similar understanding so that we treat all patches in a similar
>>> (dare I say it, consistent) manner. I've seen negative votes and positive
>>> (approved) votes for similar patches. Right now, I look at such submitted
>>> patches and ignore them, because I don't know what the fairest thing is. I
>>> don't feel right that a patch that was previously submitted gets a -2,
>>> whereas another patch gets a +A.
>>>
>>> To be clear, I think that anything that is user-facing like (log, exception)
>>> messages or our documentation should be cleaned up. (And yes, I am fine
>>> using British or American English or a mix here.)
>>>
>>> What I'm wondering about are the fixes to docstrings and inline comments
>>> that aren't externally visible.
>>>
>>> On one hand, It is great that someone submits a patch so maybe we should
>>> approve it, so as not to discourage the submitter. On the other hand, how
>>> useful are such submissions. It has already been suggested (and maybe
>>> discussed to death) that we should approve patches if there are only nits.
>>> These grammatical and misspellings fall under nits. If we are explicitly
>>> saying that it is OK to merge these nits, then why fix them later, unless
>>> they are part of a patch that does more than only address those nits?
>>>
>>> I realize that it would take me less time to approve the patches than to
>>> write this email, but I wanted to know what the community thought. Some
>>> rule-of-thumb would be helpful to me.
>>>
>>> Thoughts?
>>
>> I think improvements are improvements and we should welcome them - and
>> allow single +2/+A so long as they are not touching code.
>>
>> -Rob
>>
>>
>> --
>> Robert Collins 
>> Distinguished Technologist
>> HP Converged Cloud
>>
>> __
>> 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
> 


-- 
Sean Dague
http://dague.net

__
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] [Ironic] patches that only address grammatical/typos

2015-02-26 Thread Lucas Alvares Gomes
Hi,

I never had a strong opinion on this but reading what Jay said makes
sense to me. I also like Robert suggestion about having a single +2/+A
for such small changes.

Cheers,
Lucas

On Wed, Feb 25, 2015 at 11:22 PM, Robert Collins
 wrote:
> On 26 February 2015 at 05:26, Ruby Loo  wrote:
>> Hi,
>>
>> I was wondering what people thought about patches that only fix grammatical
>> issues or misspellings in comments in our code.
>>
>> I can't believe I'm sending out this email, but as a group, I'd like it if
>> we had  a similar understanding so that we treat all patches in a similar
>> (dare I say it, consistent) manner. I've seen negative votes and positive
>> (approved) votes for similar patches. Right now, I look at such submitted
>> patches and ignore them, because I don't know what the fairest thing is. I
>> don't feel right that a patch that was previously submitted gets a -2,
>> whereas another patch gets a +A.
>>
>> To be clear, I think that anything that is user-facing like (log, exception)
>> messages or our documentation should be cleaned up. (And yes, I am fine
>> using British or American English or a mix here.)
>>
>> What I'm wondering about are the fixes to docstrings and inline comments
>> that aren't externally visible.
>>
>> On one hand, It is great that someone submits a patch so maybe we should
>> approve it, so as not to discourage the submitter. On the other hand, how
>> useful are such submissions. It has already been suggested (and maybe
>> discussed to death) that we should approve patches if there are only nits.
>> These grammatical and misspellings fall under nits. If we are explicitly
>> saying that it is OK to merge these nits, then why fix them later, unless
>> they are part of a patch that does more than only address those nits?
>>
>> I realize that it would take me less time to approve the patches than to
>> write this email, but I wanted to know what the community thought. Some
>> rule-of-thumb would be helpful to me.
>>
>> Thoughts?
>
> I think improvements are improvements and we should welcome them - and
> allow single +2/+A so long as they are not touching code.
>
> -Rob
>
>
> --
> Robert Collins 
> Distinguished Technologist
> HP Converged Cloud
>
> __
> 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] [Ironic] patches that only address grammatical/typos

2015-02-25 Thread Robert Collins
On 26 February 2015 at 05:26, Ruby Loo  wrote:
> Hi,
>
> I was wondering what people thought about patches that only fix grammatical
> issues or misspellings in comments in our code.
>
> I can't believe I'm sending out this email, but as a group, I'd like it if
> we had  a similar understanding so that we treat all patches in a similar
> (dare I say it, consistent) manner. I've seen negative votes and positive
> (approved) votes for similar patches. Right now, I look at such submitted
> patches and ignore them, because I don't know what the fairest thing is. I
> don't feel right that a patch that was previously submitted gets a -2,
> whereas another patch gets a +A.
>
> To be clear, I think that anything that is user-facing like (log, exception)
> messages or our documentation should be cleaned up. (And yes, I am fine
> using British or American English or a mix here.)
>
> What I'm wondering about are the fixes to docstrings and inline comments
> that aren't externally visible.
>
> On one hand, It is great that someone submits a patch so maybe we should
> approve it, so as not to discourage the submitter. On the other hand, how
> useful are such submissions. It has already been suggested (and maybe
> discussed to death) that we should approve patches if there are only nits.
> These grammatical and misspellings fall under nits. If we are explicitly
> saying that it is OK to merge these nits, then why fix them later, unless
> they are part of a patch that does more than only address those nits?
>
> I realize that it would take me less time to approve the patches than to
> write this email, but I wanted to know what the community thought. Some
> rule-of-thumb would be helpful to me.
>
> Thoughts?

I think improvements are improvements and we should welcome them - and
allow single +2/+A so long as they are not touching code.

-Rob


-- 
Robert Collins 
Distinguished Technologist
HP Converged Cloud

__
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] [Ironic] patches that only address grammatical/typos

2015-02-25 Thread Sean Dague
We've unwound the gate quite a bit, so the cost of extra patches in the
merge queue fixing trivial things (like comment spelling) is pretty low.

Honestly, I'd much rather merge functional fixes faster and not go an
extra 2 rounds of typo fixing (assuming the English is decipherable),
and merge typo fixes later.

Also, if we are regularly merging typo fixes in projects, people will
actually fix the issues when they see them. If we -1 people and send
them away, they won't.

-Sean

On 02/25/2015 04:24 PM, Bernard Van De Walle wrote:
> Jay,
> I can only confirm your point of view.
> I personally landed such a patch yesterday and saw it as an easy way to
> get familiar with Gerrit.
> 
> My goal being to land some more complex patches in the near future.
> 
> Bernard
> 
> On Wed, Feb 25, 2015 at 12:37 PM, Doug Hellmann  > wrote:
> 
> 
> 
> On Wed, Feb 25, 2015, at 12:36 PM, Jay Faulkner wrote:
> >
> > > On Feb 25, 2015, at 10:26 AM, Ruby Loo  > wrote:
> > >
> > > Hi,
> > >
> > > I was wondering what people thought about patches that only fix 
> grammatical issues or misspellings in comments in our code.
> > >
> > > I can't believe I'm sending out this email, but as a group, I'd like 
> it if we had  a similar understanding so that we treat all patches in a 
> similar (dare I say it, consistent) manner. I've seen negative votes and 
> positive (approved) votes for similar patches. Right now, I look at such 
> submitted patches and ignore them, because I don't know what the fairest 
> thing is. I don't feel right that a patch that was previously submitted gets 
> a -2, whereas another patch gets a +A.
> > >
> > > To be clear, I think that anything that is user-facing like (log, 
> exception) messages or our documentation should be cleaned up. (And yes, I am 
> fine using British or American English or a mix here.)
> > >
> > > What I'm wondering about are the fixes to docstrings and inline 
> comments that aren't externally visible.
> > >
> > > On one hand, It is great that someone submits a patch so maybe we 
> should approve it, so as not to discourage the submitter. On the other hand, 
> how useful are such submissions. It has already been suggested (and maybe 
> discussed to death) that we should approve patches if there are only nits. 
> These grammatical and misspellings fall under nits. If we are explicitly 
> saying that it is OK to merge these nits, then why fix them later, unless 
> they are part of a patch that does more than only address those nits?
> > >
> > > I realize that it would take me less time to approve the patches than 
> to write this email, but I wanted to know what the community thought. Some 
> rule-of-thumb would be helpful to me.
> > >
> >
> > I personally always ask this question: does it make the software better?
> > IMO fixing some of these grammatical issues can. I don’t think we should
> > actively encourage such patches, but if someone already did the work, 
> why
> > should we run them away? Many folks use patches like this to help them
> > learn the process for contributing to OpenStack and I’d hate to run them
> > away.
> >
> > These changes tend to bubble up because they’re an easy way to get
> > involved. The time it takes to review and merge them in is an investment
> > in that person’s future interest in contributing to OpenStack, or
> > possibly open source in general.
> 
> +1
> 
> We need to keep this sort of thing in mind. If the patch is fairly
> trivial, it's also fairly trivial to review. If it is going to cause a
> more complex patch to need to be rebased, suggest that the proposer
> rebase their patch on top of the complex patch to avoid problems later
> -- that's teaching another lesson, so everyone benefits.
> 
> Doug
> 
> >
> > -Jay
> >
> >
> > > Thoughts?
> > >
> > > --ruby
> > >
> __
> > > 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)
>  

Re: [openstack-dev] [Ironic] patches that only address grammatical/typos

2015-02-25 Thread Bernard Van De Walle
Jay,
I can only confirm your point of view.
I personally landed such a patch yesterday and saw it as an easy way to get
familiar with Gerrit.

My goal being to land some more complex patches in the near future.

Bernard

On Wed, Feb 25, 2015 at 12:37 PM, Doug Hellmann 
wrote:

>
>
> On Wed, Feb 25, 2015, at 12:36 PM, Jay Faulkner wrote:
> >
> > > On Feb 25, 2015, at 10:26 AM, Ruby Loo  wrote:
> > >
> > > Hi,
> > >
> > > I was wondering what people thought about patches that only fix
> grammatical issues or misspellings in comments in our code.
> > >
> > > I can't believe I'm sending out this email, but as a group, I'd like
> it if we had  a similar understanding so that we treat all patches in a
> similar (dare I say it, consistent) manner. I've seen negative votes and
> positive (approved) votes for similar patches. Right now, I look at such
> submitted patches and ignore them, because I don't know what the fairest
> thing is. I don't feel right that a patch that was previously submitted
> gets a -2, whereas another patch gets a +A.
> > >
> > > To be clear, I think that anything that is user-facing like (log,
> exception) messages or our documentation should be cleaned up. (And yes, I
> am fine using British or American English or a mix here.)
> > >
> > > What I'm wondering about are the fixes to docstrings and inline
> comments that aren't externally visible.
> > >
> > > On one hand, It is great that someone submits a patch so maybe we
> should approve it, so as not to discourage the submitter. On the other
> hand, how useful are such submissions. It has already been suggested (and
> maybe discussed to death) that we should approve patches if there are only
> nits. These grammatical and misspellings fall under nits. If we are
> explicitly saying that it is OK to merge these nits, then why fix them
> later, unless they are part of a patch that does more than only address
> those nits?
> > >
> > > I realize that it would take me less time to approve the patches than
> to write this email, but I wanted to know what the community thought. Some
> rule-of-thumb would be helpful to me.
> > >
> >
> > I personally always ask this question: does it make the software better?
> > IMO fixing some of these grammatical issues can. I don’t think we should
> > actively encourage such patches, but if someone already did the work, why
> > should we run them away? Many folks use patches like this to help them
> > learn the process for contributing to OpenStack and I’d hate to run them
> > away.
> >
> > These changes tend to bubble up because they’re an easy way to get
> > involved. The time it takes to review and merge them in is an investment
> > in that person’s future interest in contributing to OpenStack, or
> > possibly open source in general.
>
> +1
>
> We need to keep this sort of thing in mind. If the patch is fairly
> trivial, it's also fairly trivial to review. If it is going to cause a
> more complex patch to need to be rebased, suggest that the proposer
> rebase their patch on top of the complex patch to avoid problems later
> -- that's teaching another lesson, so everyone benefits.
>
> Doug
>
> >
> > -Jay
> >
> >
> > > Thoughts?
> > >
> > > --ruby
> > >
> __
> > > 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] [Ironic] patches that only address grammatical/typos

2015-02-25 Thread Doug Hellmann


On Wed, Feb 25, 2015, at 12:36 PM, Jay Faulkner wrote:
> 
> > On Feb 25, 2015, at 10:26 AM, Ruby Loo  wrote:
> > 
> > Hi,
> > 
> > I was wondering what people thought about patches that only fix grammatical 
> > issues or misspellings in comments in our code.
> > 
> > I can't believe I'm sending out this email, but as a group, I'd like it if 
> > we had  a similar understanding so that we treat all patches in a similar 
> > (dare I say it, consistent) manner. I've seen negative votes and positive 
> > (approved) votes for similar patches. Right now, I look at such submitted 
> > patches and ignore them, because I don't know what the fairest thing is. I 
> > don't feel right that a patch that was previously submitted gets a -2, 
> > whereas another patch gets a +A.
> > 
> > To be clear, I think that anything that is user-facing like (log, 
> > exception) messages or our documentation should be cleaned up. (And yes, I 
> > am fine using British or American English or a mix here.)
> > 
> > What I'm wondering about are the fixes to docstrings and inline comments 
> > that aren't externally visible.
> > 
> > On one hand, It is great that someone submits a patch so maybe we should 
> > approve it, so as not to discourage the submitter. On the other hand, how 
> > useful are such submissions. It has already been suggested (and maybe 
> > discussed to death) that we should approve patches if there are only nits. 
> > These grammatical and misspellings fall under nits. If we are explicitly 
> > saying that it is OK to merge these nits, then why fix them later, unless 
> > they are part of a patch that does more than only address those nits?
> > 
> > I realize that it would take me less time to approve the patches than to 
> > write this email, but I wanted to know what the community thought. Some 
> > rule-of-thumb would be helpful to me.
> > 
> 
> I personally always ask this question: does it make the software better?
> IMO fixing some of these grammatical issues can. I don’t think we should
> actively encourage such patches, but if someone already did the work, why
> should we run them away? Many folks use patches like this to help them
> learn the process for contributing to OpenStack and I’d hate to run them
> away.
> 
> These changes tend to bubble up because they’re an easy way to get
> involved. The time it takes to review and merge them in is an investment
> in that person’s future interest in contributing to OpenStack, or
> possibly open source in general. 

+1

We need to keep this sort of thing in mind. If the patch is fairly
trivial, it's also fairly trivial to review. If it is going to cause a
more complex patch to need to be rebased, suggest that the proposer
rebase their patch on top of the complex patch to avoid problems later
-- that's teaching another lesson, so everyone benefits.

Doug

> 
> -Jay
> 
> 
> > Thoughts?
> > 
> > --ruby
> > __
> > 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] [Ironic] patches that only address grammatical/typos

2015-02-25 Thread Jay Faulkner

> On Feb 25, 2015, at 10:26 AM, Ruby Loo  wrote:
> 
> Hi,
> 
> I was wondering what people thought about patches that only fix grammatical 
> issues or misspellings in comments in our code.
> 
> I can't believe I'm sending out this email, but as a group, I'd like it if we 
> had  a similar understanding so that we treat all patches in a similar (dare 
> I say it, consistent) manner. I've seen negative votes and positive 
> (approved) votes for similar patches. Right now, I look at such submitted 
> patches and ignore them, because I don't know what the fairest thing is. I 
> don't feel right that a patch that was previously submitted gets a -2, 
> whereas another patch gets a +A.
> 
> To be clear, I think that anything that is user-facing like (log, exception) 
> messages or our documentation should be cleaned up. (And yes, I am fine using 
> British or American English or a mix here.)
> 
> What I'm wondering about are the fixes to docstrings and inline comments that 
> aren't externally visible.
> 
> On one hand, It is great that someone submits a patch so maybe we should 
> approve it, so as not to discourage the submitter. On the other hand, how 
> useful are such submissions. It has already been suggested (and maybe 
> discussed to death) that we should approve patches if there are only nits. 
> These grammatical and misspellings fall under nits. If we are explicitly 
> saying that it is OK to merge these nits, then why fix them later, unless 
> they are part of a patch that does more than only address those nits?
> 
> I realize that it would take me less time to approve the patches than to 
> write this email, but I wanted to know what the community thought. Some 
> rule-of-thumb would be helpful to me.
> 

I personally always ask this question: does it make the software better? IMO 
fixing some of these grammatical issues can. I don’t think we should actively 
encourage such patches, but if someone already did the work, why should we run 
them away? Many folks use patches like this to help them learn the process for 
contributing to OpenStack and I’d hate to run them away.

These changes tend to bubble up because they’re an easy way to get involved. 
The time it takes to review and merge them in is an investment in that person’s 
future interest in contributing to OpenStack, or possibly open source in 
general. 

-Jay


> Thoughts?
> 
> --ruby
> __
> 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] [Ironic] patches that only address grammatical/typos

2015-02-25 Thread Dmitry Tantsur

On 02/25/2015 05:26 PM, Ruby Loo wrote:

Hi,

I was wondering what people thought about patches that only fix
grammatical issues or misspellings in comments in our code.

I can't believe I'm sending out this email, but as a group, I'd like it
if we had  a similar understanding so that we treat all patches in a
similar (dare I say it, consistent) manner. I've seen negative votes and
positive (approved) votes for similar patches. Right now, I look at such
submitted patches and ignore them, because I don't know what the fairest
thing is. I don't feel right that a patch that was previously submitted
gets a -2, whereas another patch gets a +A.

/me too



To be clear, I think that anything that is user-facing like (log,
exception) messages or our documentation should be cleaned up. (And yes,
I am fine using British or American English or a mix here.)

What I'm wondering about are the fixes to docstrings and inline comments
that aren't externally visible.

On one hand, It is great that someone submits a patch so maybe we should
approve it, so as not to discourage the submitter. On the other hand,
how useful are such submissions. It has already been suggested (and
maybe discussed to death) that we should approve patches if there are
only nits. These grammatical and misspellings fall under nits. If we are
explicitly saying that it is OK to merge these nits, then why fix them
later, unless they are part of a patch that does more than only address
those nits?
The biggest problem is that these patches 1. take our time, 2. take gate 
resources, 3. may introduce merge conflicts.


So I would suggest agree to -2 patches that fix _only_ user-invisible 
strings.




I realize that it would take me less time to approve the patches than to
write this email, but I wanted to know what the community thought. Some
rule-of-thumb would be helpful to me.

Thoughts?

--ruby


__
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] [Ironic] patches that only address grammatical/typos

2015-02-25 Thread Alexis Lee
Ruby Loo said on Wed, Feb 25, 2015 at 11:26:56AM -0500:
> I was wondering what people thought about patches that only fix grammatical
> issues or misspellings in comments in our code.

For my money, a patch fixing nits has value but only if it fixes a few.
If it's a follow-up patch it should fix all the nits; otherwise it
should be of a significant chunk, EG a file, class or large method.

> It has already been suggested (and maybe
> discussed to death) that we should approve patches if there are only nits.
> These grammatical and misspellings fall under nits. If we are explicitly
> saying that it is OK to merge these nits, then why fix them later, unless
> they are part of a patch that does more than only address those nits?

We'd rather they were fixed though, right? Letting patches with nits
land is a pragmatic response to long response times or social friction.
Likewise a follow-up patch to fix nits can be a very pragmatic way to
allow the original patch to land quickly without sacrificing code
readability.


Alexis
-- 
Nova Engineer, HP Cloud.  AKA lealexis, lxsli.

__
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] [Ironic] patches that only address grammatical/typos

2015-02-25 Thread Ruby Loo
Hi,

I was wondering what people thought about patches that only fix grammatical
issues or misspellings in comments in our code.

I can't believe I'm sending out this email, but as a group, I'd like it if
we had  a similar understanding so that we treat all patches in a similar
(dare I say it, consistent) manner. I've seen negative votes and positive
(approved) votes for similar patches. Right now, I look at such submitted
patches and ignore them, because I don't know what the fairest thing is. I
don't feel right that a patch that was previously submitted gets a -2,
whereas another patch gets a +A.

To be clear, I think that anything that is user-facing like (log,
exception) messages or our documentation should be cleaned up. (And yes, I
am fine using British or American English or a mix here.)

What I'm wondering about are the fixes to docstrings and inline comments
that aren't externally visible.

On one hand, It is great that someone submits a patch so maybe we should
approve it, so as not to discourage the submitter. On the other hand, how
useful are such submissions. It has already been suggested (and maybe
discussed to death) that we should approve patches if there are only nits.
These grammatical and misspellings fall under nits. If we are explicitly
saying that it is OK to merge these nits, then why fix them later, unless
they are part of a patch that does more than only address those nits?

I realize that it would take me less time to approve the patches than to
write this email, but I wanted to know what the community thought. Some
rule-of-thumb would be helpful to me.

Thoughts?

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