===== 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.