On Wed, Feb 25, 2015, at 12:36 PM, Jay Faulkner wrote: > > > On Feb 25, 2015, at 10:26 AM, Ruby Loo <[email protected]> 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: [email protected]?subject:unsubscribe > > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > > __________________________________________________________________________ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: > [email protected]?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: [email protected]?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
