Merge lp:~eric97/bzr/fix-merge-docs into lp:bzr

[Eric Siegerman]

Clarify the description of merge's "--revision" option.
Plus a drive-by fix.

A question came up today on the mailing list as to which merges are cherrypicks. Part of the problem is that the description of --revision is a bit muddy. This attempts to clarify that.

[Vincent Ladeuil (vila)]

I'm not a native English speaker, but this is far clearer to me.
I'm not a lawyer either so I'm not sure we need you to execute the Canonical's Contributor Agreement: <http://www.canonical.com/contributors>.

But despite these hesitations, I approve this with both hands :)

We need a second reviewer still (which may not be a lawyer either but has good chances to be a native ;).

review: Approve

[John C Barstow (jbowtie)]

", automatically determining an appropriate base revision. If this
fails, you may need to give an explicit base."

In what situation would I need to do that? Is there a specific error
message? If we don't have specific guidance I would remove this from
the builtins (though it may be appropriate in the user guide).

"To pick a different ending revision, pass "--revision OTHER". Bzr
will try to merge in all new work up to and including revision OTHER."

I would probably say: To merge changes up to and including a specific
revision, use "--revision".

+ If you specify two values, "--revision BASE..OTHER", only revisions BASE
+ through OTHER, excluding BASE but including OTHER, will be merged. If this
+ causes some revisions to be skipped, i.e. if the destination branch does
+ not already contain revision BASE-1, such a merge is commonly referred to
+ as "cherrypicking".

This is pretty unclear. Cherrypicking is usually defined as merging in
a specific range of revisions. Conceptually you are merging the
*changes* between BASE and OTHER. The details about whether or not
revisions are skipped should probably be in the user guide, if
anywhere as conceptually it doesn't matter.

[Matthew Fuller (fullermd)]

> + If you specify two values, "--revision BASE..OTHER", only revisions BASE
> + through OTHER, excluding BASE but including OTHER, will be merged. If this
> + causes some revisions to be skipped, i.e. if the destination branch does
> + not already contain revision BASE-1, such a merge is commonly referred to
> + as "cherrypicking".

This is incorrect; it's a cheerypick unless the destination already
includes *BASE*, not *BASE-1*.

[Matthew Fuller (fullermd)]

> This is pretty unclear. Cherrypicking is usually defined as merging
> in a specific range of revisions.

It's not merging a specific range that makes it a cherrypick; it's
that the range is _disconnected_ from the revs you already have.

[Eli Zaretskii (eliz)]

> ", automatically determining an appropriate base revision. If this
> fails, you may need to give an explicit base."
>
> In what situation would I need to do that? Is there a specific error
> message? If we don't have specific guidance I would remove this from
> the builtins (though it may be appropriate in the user guide).
>
> "To pick a different ending revision, pass "--revision OTHER". Bzr
> will try to merge in all new work up to and including revision OTHER."
>
> I would probably say: To merge changes up to and including a specific
> revision, use "--revision".
>
> + If you specify two values, "--revision BASE..OTHER", only revisions BASE
> + through OTHER, excluding BASE but including OTHER, will be merged. If
> this
> + causes some revisions to be skipped, i.e. if the destination branch does
> + not already contain revision BASE-1, such a merge is commonly referred to
> + as "cherrypicking".
>
> This is pretty unclear. Cherrypicking is usually defined as merging in
> a specific range of revisions. Conceptually you are merging the
> *changes* between BASE and OTHER. The details about whether or not
> revisions are skipped should probably be in the user guide, if
> anywhere as conceptually it doesn't matter.

The modified documentation looks very clear to me, including the last part. It actually correctly defines cherrypicking as merging revisions that break the sequence of revisions from the other branch. So I think it's very useful, and having this right there with "merge" docs is exactly where it's needed.

[Eric Siegerman (eric97)]

> > + If you specify two values, "--revision BASE..OTHER", only revisions
> BASE
> > + through OTHER, excluding BASE but including OTHER, will be merged. If
> this
> > + causes some revisions to be skipped, i.e. if the destination branch
> does
> > + not already contain revision BASE-1, such a merge is commonly referred
> to
> > + as "cherrypicking".
>
> This is incorrect; it's a cheerypick unless the destination already
> includes *BASE*, not *BASE-1*.

Fixed. Thanks for catching it.

[Eric Siegerman (eric97)]

> ", automatically determining an appropriate base revision. If this
> fails, you may need to give an explicit base."
>
> In what situation would I need to do that? Is there a specific error
> message? If we don't have specific guidance I would remove this from
> the builtins (though it may be appropriate in the user guide).

To be honest, I don't know -- which means I'm hardly competent to explain it (though I agree with you that more explanation would be useful). So I'd rather treat that as out of scope for this MP. (I didn't write the "If this fails" sentence; it was already in the text. I merely moved it to its current location along with the rest of that paragraph.)

> "To pick a different ending revision, pass "--revision OTHER". Bzr
> will try to merge in all new work up to and including revision OTHER."
>
> I would probably say: To merge changes up to and including a specific
> revision, use "--revision".

I specifically wanted to show the one-revision syntax, to contrast it with the two-revision syntax to follow; and also to define OTHER before using it. That said, I'm not thrilled with "OTHER" as the variable name; I used it because it was in the old version (where it wasn't defined, by the way).

> + If you specify two values, "--revision BASE..OTHER",j
> + [...]
> + such a merge is commonly referred to
> + as "cherrypicking".
>
> This is pretty unclear.

What fullermd and Eli said :-/

[Gordon Tyler (doxxx)]

Looks good to me, from a native speaker point of view.

review: Approve

[Andrew Bennetts (spiv)]

Looks ok to me too. I'd be happy to land this for you, except I don't think you've executed the Canonical Contributor Agreement yet. I'm pretty sure we require a contributor agreement for this sort of change (even though it's a relatively small update to help text).

[Andrew Bennetts (spiv)]

I'm going to mark this Work in Progress pending the Contributor Agreement, to move it off of the activereviews page. Let us know when you've executed that agreement and we can get this merged. (Or let us know if you won't or can't.)

[Martin Pool (mbp)]

Eric sent the CA; thanks.

http://wiki.bazaar.canonical.com/Branding says 'bzr' is lowercase, even at the start of a sentence.

I'll tweak that and land this if there's nothing else people want changed.

review: Approve

[lp:~eric97/bzr/fix-merge-docs]

5541. By Eric Siegerman <email address hidden> on 2010-11-30

Fix capitalization of "bzr", per review. Grammar fix: change "cherrypicking" to "a cherrypick".

[Martin Pool (mbp)]

sent to pqm by email

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2010-11-18 00:22:24 +0000
+++ bzrlib/builtins.py	2010-11-30 04:54:11 +0000
@@ -3749,16 +3749,20 @@
     with bzr send. If neither is specified, the default is the upstream branch
     or the branch most recently merged using --remember.
 
-    When merging a branch, by default the tip will be merged. To pick a different
-    revision, pass --revision. If you specify two values, the first will be used as
-    BASE and the second one as OTHER. Merging individual revisions, or a subset of
-    available revisions, like this is commonly referred to as "cherrypicking".
-
-    Revision numbers are always relative to the branch being merged.
-
-    By default, bzr will try to merge in all new work from the other
-    branch, automatically determining an appropriate base.  If this
-    fails, you may need to give an explicit base.
+    When merging from a branch, by default bzr will try to merge in all new
+    work from the other branch, automatically determining an appropriate base
+    revision.  If this fails, you may need to give an explicit base.
+
+    To pick a different ending revision, pass "--revision OTHER".  bzr will
+    try to merge in all new work up to and including revision OTHER.
+
+    If you specify two values, "--revision BASE..OTHER", only revisions BASE
+    through OTHER, excluding BASE but including OTHER, will be merged.  If this
+    causes some revisions to be skipped, i.e. if the destination branch does
+    not already contain revision BASE, such a merge is commonly referred to as
+    a "cherrypick".
+
+    Revision numbers are always relative to the source branch.
 
     Merge will do its best to combine the changes in two branches, but there
     are some kinds of problems only a human can fix.  When it encounters those,
@@ -3788,7 +3792,7 @@
     you to apply each diff hunk and file change, similar to "shelve".
 
     :Examples:
-        To merge the latest revision from bzr.dev::
+        To merge all new revisions from bzr.dev::
 
             bzr merge ../bzr.dev