Open Stack

On 24/02/15 09:00, Nick Chase wrote:
>
> On 2/23/2015 1:52 PM, Andreas Jaeger wrote:
>> Thanks for bringing it up. We do have guidelines
>> here: https://wiki.openstack.org/wiki/Documentation/ReviewGuidelines so
>> feel free to enhance those to help solve the problem.
>> Before we do this, I suggest that we experiment a bit with the workflow.
>> Nick, could you show us a few examples how this would work, please?
>
> Basically, as far as the ReviewGuidelines, we would add a new type of
> change, making the three possibilities "Objective", "Subjective", and
> "Cosmetic/Convention".  So for a (probably silly, but demonstrative)
> example:
>
> -- You submit a change that adds a new section talking about how to add,
> say, a new driver for Cinder, and everywhere you should have "OpenStack
> Block Storage" you just say "cinder".  You a
> -- I review your change, and I see that everything is fine, except for
> those 17 instances of "cinder".
> -- From here it can go one of three ways:
> -- 1) I comment that I'm going to fix it (so you don't spend time on
> it), then correct them and submit a patch, and +1.
> -- 2) I mark them, and you fix them and resubmit.  I +1.
> -- 3) I let you know that it needs to be fixed and:
> -- -- 3a) you submit a bug and drop it into comments, then I +1 or
> -- -- 3b) I submit a bug, then +1

So, this isn't totally insane. From where I'm sitting though, I'd vastly 
prefer to see 1 and 2 happen in 90%+ cases. I think we need to be having 
a broader discussion about review rigour in general, though, so we can 
better identify where a -1 isn't warranted, where a reviewer should do a 
quick fix on someone else's patchset, or where a new bug is a better way 
of fixing issues.

I know when I started working on OpenStack docs, doing reviews was quite 
daunting, mostly because I didn't have a good idea what level of review 
rigour was expected. Here's some notes I wrote about that at the time:

> A note on review rigour: There are very few guidelines about what consists of a successful patch, but the general approach seems to be that if it's technically accurate and better than the existing content, then it should be approved. The main things to look for:
> General spelling and grammar.
> Technical accuracy. Where possible, test commands on your own VM to make sure they're accurate. Check any related bugs, and the Disqus comments on the doc site to see if there's anything else you might need to take into account.
> The 'is it better than what we have already' test. Check the diff, or go look at the current document on the doc site, and determine if the changes are an improvement.
> Provide corrections in-line (double click on the offending line in the diff viewer to write your suggested changes) for the author to fix if there's more than a couple of errors. If there's just one or two really minor changes (or in a situation where the writer is either ESL or could be otherwise unable to improve the doc themselves), consider checking out the patch and editing it quickly yourself. Be nice.

Hopefully this can start a broader conversation about what is/isn't 
expected from reviewers :)

L

-- 
Lana Brindley
Technical Writer
Rackspace Cloud Builders Australia
http://lanabrindley.com