[m-rev.] for review: Update reviews document

Julien Fischer jfischer at opturion.com
Wed Feb 12 17:30:14 AEDT 2014


Hi,

On Wed, 12 Feb 2014, Paul Bone wrote:

> Update reviews document
>
> We now use git and github rather than CVS.  I've updated our code review
> process document to refer to git and it's workflow rather than CVS.

s/it's/its/

> I've also fixed some spelling errors and changed some formatting.
>
> compiler/notes/reviews.html:
>    As above.
> ---
> compiler/notes/reviews.html | 118 +++++++++++++++++++++++++-------------------
> 1 file changed, 68 insertions(+), 50 deletions(-)
>
> diff --git a/compiler/notes/reviews.html b/compiler/notes/reviews.html
> index c4976d2..9e49b84 100644
> --- a/compiler/notes/reviews.html
> +++ b/compiler/notes/reviews.html
> @@ -37,64 +37,83 @@ need to be reviewed.
> <ol>
> <li>  Make sure you are working with an up-to-date copy of the
> 	    module you are using.
> -<li>  If change is a code change, test change. See "Testing" section
> +<li>  If the change is a code change, test change. See "Testing" section
> 	    of coding standards. Testing may take time - don't forget
> 	    that steps 3, 4 and 5 can be done in parallel.
> -<li>  Create diff - use `cvs diff -u'.  New files should be
> +<li>  Write a log message for this change - use a template (see below).  How
> +        you include this will depend on the next step.
> +<li>  Create a diff.
> +        There are two ways to do this, you can either use git and github to
> +        create a pull request (see
> +        <a href="https://help.github.com/articles/using-pull-requests">Github's
> +        documentation</a>,
> +        or create one or more changes and mail them to us using either
> +        <code>git diff</code> or by creating series of commits and using
> +        <code>git format-patch</code> (see the git documentation for
> +        details).
> +        You will need to edit the files that format-patch generates to
> +        modify the subject line (see below), and to duplicate this line
> +        within the e-mail's body.
> +        If you create a diff then new files should be
> 	    appended verbatim to the end of the diff, with descriptions
> 	    indicating the name of the file.
> -<li>  Write log message for this change - use template (see below).
> <li>  Review diff and log message yourself. (see below)
> -<li>  Send to mercury-reviews at cs.mu.oz.au, with the subject
> -	    "for review: <short description of change>".
> -	    Nominate a reviewer at top of diff (see below).
> +<li>  Submit the pull request or mail the changes to us at
> +      <a href="mailto:Mercury Reviews <reviews at lists.mercurylang.org>">
> +       Mercury Reviews <reviews at lists.mercurylang.org></a>

While I don't wish to discourage (potential) contributers, my preference
is to use the mailing list.  Maybe, this should mention that is the
preferred way of submitting changes.
(Is it possible for github to automagically post pull requests to the
reviews list?)

> +       The subject should be
> +	    "for review: <short description of change>", the short
> +        description is typically the first line of your git log message.
> +	    Optionally nominate a reviewer at top of diff (see below).
> +        Optionally describe which branches your change should be applied to.
> 	    (If this change has been reviewed once before, it might
> 	    fall into the "commit before review" category -- see the
> 	    section on exceptions).
> +      If you generated your diffs using <code>git format-patch</code> then
> +      you can use <code>git send-email</code> to mail them, or attach them
> +      to an e-mail normally.
> <li>  Wait for review (see below).
> -<li>  Fix any changes suggested.
> +<li>  Fix any changes suggested.

I suggest s/Fix/Make/ there.

The rest looks ok.

Cheers,
Julien.



More information about the reviews mailing list