Bit of review for svnbook

Stefan Sperling stsp at elego.de
Sat Jun 7 11:32:43 CDT 2008


Hi,

I've reviewed these sections:

What's a Branch?
Using Branches
Basic Merging 

in the multiple page html version compiled from r3106.

I only have remarks on the "Basic Merging" section, the
others are fine.

-----------------------------

There is an error in the example output of "svn status" right
after the first example merge is done:

 $ svn status
 M      .
 M      button.c
 M      integer.c
 
Right below that, it says:

 At this point, the wise thing to do is look at the changes carefully
 with svn diff, and then build and test your branch. Notice that the
 current working directory (“.”) has also been modified; the svn diff
 will show that its svn:mergeinfo property has been either created or
 modified.

Since there is a property modification on '.', shouldn't the M appear
in the second column?

 $ svn status
  M     .
 M      button.c
 M      integer.c
 
A small test merge I did indicates so:

 $ svn merge file:///tmp/repos/trunk
 --- Merging r2 through r3 into '.':
 A    a
 $ svn st
  M     .
 A  +   a

-----------------------------

I also have a remark on the following sentence which occurs in the
same section:

 If you encounter serious problems, you can always abort the local
 changes by running svn revert and start a long “what's going on?”
 discussion with your collaborators. 

I think this is too terse, because "svn revert path/to/some/file"
does not revert the mergeinfo created during the merge. Trying to
merge the file again to resurrect it will lead to Subversion not
doing anything, because acording to the mergeinfo, the file has
already been merged. I guess quite a few people will naively try
"svn revert <file>" after reading the above, though. If they do
this, they will not be able to merge the file into their working
copy again.

I've adopted the habit of reverting my whole working copy recursively
when reverting botched merges:

  cd working-copy-root
  svn revert -R .

This makes sure that the svn:mergeinfo property is also reverted.
I don't think we have a way yet to remove only certain revs from the
mergeinfo (well, apart from manually editing the property, but we don't
want to suggest doing that in the book, no no no ;)

"svn revert -R" is mentioned later in the same section, but there's no
hint made at the quirks of the interaction of svn:mergeinfo and
"svn revert":

 If you don't like the results of the merge, simply svn revert -R the
 changes from your working copy and retry the command with different
 options. The merge isn't final until you actually svn commit the
 results.

-----------------------------

Beneath this figure:

 $ cd my-calc-branch
 $ svn propget svn:mergeinfo .
 /trunk:341-390

the text says:

 In general, this property is automatically maintained by Subversion
 whenever you run svn merge, and its value indicates which changes have
 already been replicated into a particular directory.

I think this would be more accurate and help users understand better
what exactly these properties represent:

 In general, this property is automatically maintained by Subversion
 whenever you run svn merge. Its value indicates which changes made
 at a given path have already been replicated into the directory which
 the svn:mergeinfo property is attached to. In our case, the path is
 "/trunk", and the directory which the property is attached to is
 "/branches/my-calc-branch".

-----------------------------

In this figure:

 $ svn merge -c -303 http://svn.example.com/repos/calc/trunk
 --- Reverse-merging r303 into 'integer.c':
 U    integer.c
 
 $ svn status
 M      integer.c
 
 $ svn diff
 …
 # verify that the change is removed
 …
 
 $ svn commit -m "Undoing change committed in r303."
 Sending        integer.c
 Transmitting file data .
 Committed revision 350.

shouldn't the output of "svn status" include an svn:mergeinfo modification?
Like so:

 $ svn status
  M     .
 M      integer.c
 
It does in my test:

$ svn merge -c -3 file:///tmp/repos/trunk 
--- Reverse-merging r3 into '.':
D    a
$ svn st
 M     .
D      a
$ svn diff --depth=empty

Property changes on: .
___________________________________________________________________
Modified: svn:mergeinfo
   Reverse-merged /trunk:r3



That's it for now :)

Stefan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <http://www.red-bean.com/pipermail/svnbook-dev/attachments/20080607/45a5d31d/attachment.sig>


More information about the svnbook-dev mailing list