<div dir="ltr"><div><div><div>Jay, <br>I can only confirm your point of view.<br></div>I personally landed such a patch yesterday and saw it as an easy way to get familiar with Gerrit.<br><br></div>My goal being to land some more complex patches in the near future.<br><br></div>Bernard<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Feb 25, 2015 at 12:37 PM, Doug Hellmann <span dir="ltr"><<a href="mailto:doug@doughellmann.com" target="_blank">doug@doughellmann.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
<br>
On Wed, Feb 25, 2015, at 12:36 PM, Jay Faulkner wrote:<br>
><br>
> > On Feb 25, 2015, at 10:26 AM, Ruby Loo <<a href="mailto:rlooyahoo@gmail.com">rlooyahoo@gmail.com</a>> wrote:<br>
> ><br>
> > Hi,<br>
> ><br>
> > I was wondering what people thought about patches that only fix grammatical issues or misspellings in comments in our code.<br>
> ><br>
> > 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.<br>
> ><br>
> > 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.)<br>
> ><br>
> > What I'm wondering about are the fixes to docstrings and inline comments that aren't externally visible.<br>
> ><br>
> > 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?<br>
> ><br>
> > 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.<br>
> ><br>
><br>
> I personally always ask this question: does it make the software better?<br>
> IMO fixing some of these grammatical issues can. I don’t think we should<br>
> actively encourage such patches, but if someone already did the work, why<br>
> should we run them away? Many folks use patches like this to help them<br>
> learn the process for contributing to OpenStack and I’d hate to run them<br>
> away.<br>
><br>
> These changes tend to bubble up because they’re an easy way to get<br>
> involved. The time it takes to review and merge them in is an investment<br>
> in that person’s future interest in contributing to OpenStack, or<br>
> possibly open source in general.<br>
<br>
</span>+1<br>
<br>
We need to keep this sort of thing in mind. If the patch is fairly<br>
trivial, it's also fairly trivial to review. If it is going to cause a<br>
more complex patch to need to be rebased, suggest that the proposer<br>
rebase their patch on top of the complex patch to avoid problems later<br>
-- that's teaching another lesson, so everyone benefits.<br>
<span class="HOEnZb"><font color="#888888"><br>
Doug<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
><br>
> -Jay<br>
><br>
><br>
> > Thoughts?<br>
> ><br>
> > --ruby<br>
> > __________________________________________________________________________<br>
> > OpenStack Development Mailing List (not for usage questions)<br>
> > Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
> > <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
><br>
> __________________________________________________________________________<br>
> OpenStack Development Mailing List (not for usage questions)<br>
> Unsubscribe:<br>
> <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</div></div></blockquote></div><br></div>