Review workflow
I think peer reviews are excellent tools to get the best code/documentation in our repositories. In this page I describe my review workflow for both doc and src repositories which are already migrated to git.
Creating the review
First thing I do is to update the main branch of the repo:
git checkout main git pull --ff-only
Git branches are cheap so I create branches for the new features/fixes:
git branch -b netstat_add_examples
This will create a new branch called netstat_add_examples and switch to it. Now it is time to do some changes. Assume that I changed the file usr.bin/netstat/netstat.1. We add it to the stage area and commit it.
git add usr.bin/netstat/netstat.1 $ git commit -m 'netstat(1): Add EXAMPLES to man page'
Now we can create our revision. In order to do that, we need to tell arc that we want to create the review with the changes that we have in our branch (netstat_add_examples) with respect to main:
arc diff --create main
We write the review title and comments in the editor that shows up and we are done!
Making changes to the review
Time pases and a fellow committer picks up the review and ask you to make some changes. We discuss it and agree to those changes, so we do it in a normal way:
git checkout netstat_add_examples vim usr.bin/netstat/netstat.1 git add usr.bin/netstat/netstat.1 git commit -m 'Rework first example as requested by fellow1@'
and update the review:
arc diff --update DXXXXX main
After some time, another fellow committer spots a typo. We fix it:
git checkout netstat_add_examples vim usr.bin/netstat/netstat.1 git add usr.bin/netstat/netstat.1 git commit -m 'Fix typo spotted by fellow2@'
and update the review again:
arc diff --update DXXXXX main
And this time, fellow2 accepts the review.
Committing the accepted review
We have our changes sitting on the netstat_add_examples branch. Unfortunately, we didn't do everything right at the first attempt (that is what reviews are for) so now we have three commits to push to the repository. But we don't want to push the three commits since it does not make sense to do a commit that we know contains a typo and do a follow up with the fix for that typo. Fortunately, we can do it in just one commit. The first thing to do is to rebase our branch with main. Maybe days have passed since we created the review so we need to update our local repositories. First, update main:
git checkout main git pull --ff-only
Now, we want to get all those new changes from main in our netstat_add_examples branch and then, on top of that, put our changes:
git rebase -i main netstat_add_examples
Then, git will open an editor and show something like this:
pick 1344d6002eeb netstat(1): netstat(1): Add EXAMPLES to man page pick e784a0ecf126 netstat(1): Rework first example as requested by fellow1@ pick e7b4a0eff154 netstat(1): Fix typo spotted by fellow2@
The first column are commands understood by git. If we change the text above, to this:
pick 1344d6002eeb netstat(1): netstat(1): Add EXAMPLES to man page squash e784a0ecf126 netstat(1): Rework first example as requested by fellow1@ squash e7b4a0eff154 netstat(1): Fix typo spotted by fellow2@
git will do only one commit (the first one) but with the accumulated changes of the three commits. When we save the file and exit, git opens up an editor and shows the commit messages of the three commits. We can now tailor the message to our needs, adding or removing what we want. When we save the file and exit the editor, the commit is made. If we now do:
git log
we will see only one commit, but if we do:
git log -c
we can verify that it contains the changes of all three commits.
Now the only thing left is to push the changes to the repository. There is no need to merge the netstat_add_examples branch to main, we can push it directly:
git push freebsd netstat_add_examples:main
Committing the accepted review (alternative workflow)
Let's say that we created the review, and did like five or six new commits and review updates to incorporate the changes suggested by the reviewers. An alternative to the previous workflow would be to ask arc to apply the patch for us. It is smart enough to do it in a new branch and all so it is pretty convenient. First, let us update our main branch:
git checkout main git pull --ff-only
And now, ask arc to apply the patch:
$ arc patch DXXXXX
INFO Base commit is not in local repository; trying to fetch.
Enter passphrase for key '/home/fernape/.ssh/id_rsa':
Created and checked out branch arcpatch-DXXXXX.
This diff is against commit svn+ssh://repo.freebsd.org/base/head@368025,
but the commit is nowhere in the working copy. Try to apply it against
the current working copy state?
(109260d202fb64be6f2efcf243c25090c1f64420) [Y/n] Y
...arc applied the patch in the branch arcpatch-DXXXXX. At this point, you have a new commit with all the changes from the review (from the 5 or 6 review updates we made). arc used the title and description of the review to create the commit message. This is probably not what you need so you may want to change it:
git commit --amend
Rewrite the commit message and save the file.
So now, we only need to push this new branch:
git push freebsd arcpatch-DXXXXX:main
In this alternative path, we still used netstat_add_examples as the work-in-progress branch and then used the arcpatch-DXXXXX branch to get all the changes from the review in one single commit. At the cost of having two branches, this workflow might be more convenient for some people.