1098265 - MozReview documents needs details on how to pull patches for review

Status: RESOLVED FIXED

(MozReview Graveyard :: General, defect, P1)

Description

[Mark Banner (:standard8)]

The MozReview documents (http://mozilla-version-control-tools.readthedocs.org/en/latest/mozreview.html) really could do with some information on how to do a pull and a review, and get back to your original state.

On the bugs, you just get:

====
Pull down this commit:

hg pull review -r 9fe37f5753287a3d72cdbe06af68a65ffefab0b1
====

but its not entirely clear/documented anywhere any easy ways of doing that in such a way that you get updated to that revision, and that it doesn't affect hg outgoing, or supply some other way of getting back to the original state easily.

Comment 1

[Dan Minor [:dminor]]

Attachment: New section on how to work with patches

I've added some sections on basic patch manipulation. Please let me know if there are any other topics you'd like to see discussed. Thanks!

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8616833 [details] [diff] [review]
New section on how to work with patches

Review of attachment 8616833 [details] [diff] [review]:
-----------------------------------------------------------------

::: docs/mozreview/reviewboard.rst
@@ +207,5 @@
> +   to go.
> +
> +   To pull the commits down use the url provided in the review
> +   description, but avoiding doing an update at the same time, which will force 
> +   a merge. For instance, if the revision is foo::

'foo':: may be clearer

@@ +238,5 @@
> +
> +   If you're using queues, you can pop it from your queue and then delete it::
> +
> +   $ hg qpop 
> +   $ hg qdelete foo.diff

Is it always named foo.diff (where foo is the revision)?

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8616833 [details] [diff] [review]
New section on how to work with patches

Review of attachment 8616833 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me as well. Thanks!

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 8616833 [details] [diff] [review]
New section on how to work with patches

Review of attachment 8616833 [details] [diff] [review]:
-----------------------------------------------------------------

::: docs/mozreview/reviewboard.rst
@@ +200,4 @@
>     to offer little value over just going to Review Board and looking at
>     the original comments there.)
>  
> +Working With "Patches"

There should be === or --- under here to create a new section.

@@ +207,5 @@
> +   to go.
> +
> +   To pull the commits down use the url provided in the review
> +   description, but avoiding doing an update at the same time, which will force 
> +   a merge. For instance, if the revision is foo::

``foo`` will do monospace/<code> magic is almost certainly what you want for inline identifiers.

Also, `hg pull` + `hg update` will *not* do a merge unless you have uncommitted local changes, in which case the pull is irrelevant to a merge occurring.

@@ +219,5 @@
> +
> +   Or import it into a mercurial queue::
> +
> +   $ hg qimport -r foo
> +   $ hg qapplied

We should strive for no new MQ references in Mercurial documentation, as MQ is an unloved feature and only traps people into sub-optimal workflows.

@@ +230,5 @@
> +
> +   $ hg export qtip > foo.patch
> +
> +   To remove the patch once you've finished, assuming you're using bookmarks,
> +   you can strip it from the repository and then delete the bookmark::

I'm not a fan of these instructions. Extra heads are harmless. You should never need to strip your repo. Learn how to use multiple heads. https://mozilla-version-control-tools.readthedocs.org/en/latest/hgmozilla/bookmarks.html#understanding-head-based-development

Comment 5

[Dan Minor [:dminor]]

Thanks for your feedback, I've made the fixes you suggested.

> 
> We should strive for no new MQ references in Mercurial documentation, as MQ
> is an unloved feature and only traps people into sub-optimal workflows.
> 
I disagree, I've added some words to indicate that using bookmarks is the preferred workflow, but I think it's important to provide both sets of instructions. Conversations I've had have indicated that people are more likely to try bookmarks if there is a side-by-side view of how to do things with queues and with bookmarks. This can be done in blog posts, but it is much more discoverable in the docs.

Comment 6

[Dan Minor [:dminor]]

Rather than delay further I've pushed the updates to [1]. We can always revise later.

[1] https://hg.mozilla.org/hgcustom/version-control-tools/rev/364e671fcbe3