HOWTO: review patches

Marcel van der Boom <[email protected]> Fri, 06 Dec 2002 07:24:35 +0100
Newsgroups gmane.comp.cms.xaraya.patches
Organization HS-Development BV
Message-ID <[email protected]>
A small howto for reviewing patches. I'll use the patch submitted by 
richard as an example.

After the patch was submitted I checked in the patch on the server. 
So the patch was stored in: PENDING/patch-2002-12-04-18-53-46

Patch review steps:

1. bk takepatch -f PENDING/patch-2002-12-04-18-53-46

This will create a RESYNC dir in the root of your repository which contains 
a 'mini-repository' with all the changed files in the patch with their new 
contents.

The normal rules apply in the same way as if you were doing a normal pull 
(bk exchanges information by sending patches, only normally you don't see 
much of them) To understand bk pull basically puts things in the PENDING 
directorry and then calls bk takepatch and bk resolve on all of them. The 
patch system is one level lower than that, it just puts stuff in the PENDING
directory for review.

2. Reviewing the patch

The RESYNC dir contains a mini-repository, so all commands which work in 
the repository also apply to the RESYNC dir, they get a different meaning 
in there.

The quickest way to do a review is issueing: bk csets

This command will analyze the latest csets which came in, which is the 
patch you 'took' in step 1. 

It will give you a window with the changeset, it's comments and the diff of 
each file in the changeset. For content review I find this the most 
convenient tool.

What you don't see in this tool is the context of the changeset.

3. Analyzing consequences

If you suspect this changeset will have consequences you can do the 
following:

cd RESYNC   -- go into the mini repository
bk revtool  -- view history of the tree with patch put into context

The last command will give you a tree like picture which will be a lot like 
the diagrams in rfc28. The RESYNC tree has knowledge of the whole tree. 
The patch which you are reviewing is always a 'leaf' in the tree. This 
means that (depending how many changesets were pushed after the patch) that 
it will be on the top line of the graph as the last node. All subsequent 
changesets are automatically created on a branch by bitkeeper and split off 
before the patch. If more patches are pending you will see more branches in 
the graph and more 'end-nodes'. 

To use the example of richards patch: 
The revision of the patch would be 1.329, which follows 1.328
As changeset 1.329 is not applied yet, bk creates a branch and all 
subsequent changesets are created on the branch 1.328.1. As you can see 
with revtool the changeset 1.328.1.1 was the next one which was applied to
the tree. (neat no?, imagine how this would work in cvs)
By clicking on the changeset blocks in the graph you can see the comments 
entered by the changeset and get a feeling what was changed by them.
See bk help revtool for other options in this tool (there are many)

4. Applying the patch

If you feel that the patch can be applied close the revtool and go back to 
the root of the repository

bk resolve

applies the patch and you get the changes to resolve conflicts if any.

Wether you are applying or rejecting the patch, that doesn't matter, this 
step is always necessary. If conflicts are resolved and applied as well, 
you can do the bk push and you're done. The submitter will receive his 
changes on his next pull. (they will automatically resolve in his tree, 
because his repository already has the changes)


5. Reject the patch

After applying the patch in step 4 the following command will reject the 
patch, explicitly registering this in the tree

bk cset -x<revno>

The command means: create a changeset at the top of the tree (TOT) which is 
the reverse of changeset <revno>.


I have left the patch in the PENDING directory so you can actually try this 
out yourself. 
Remember that the changeset revision numbers are not fixed, so you might 
get other revision numbers in your revtool window.

Marcel

-- 
Marcel van der Boom, HS-Development BV
Kwartiersedijk 14B Fijnaart, The Netherlands
T: +31-168-468822, F: 0168-468823, E: [email protected]