Some hints and practices related to code review via http://codereview.appspot.com/. Most part of it related to hg usage in connection with it (so it may be applied to another CR tool with CLI).
Initial steps
Mostly described in http://moinmo.in/MoinDev/MercurialGuide
Setup hgrc
For making work with hg productive, it's recommended (and needed for some features mentioned below) to enable number of plugins. Example [extensions] section of ~/.hgrc config file:
[extensions] histedit = ~/.hg/histedit/hg_histedit.py mq = rebase = record = color =
Appropriate links describing extensions:
tl;dr
(short summary of the next section)
- Commit frequently
Before commit, check hg status -q and hg diff, check tests with pytest, than use hg commit file1 fileN -m "Commit message" or hg record to commit.
After work is done, review/edit your commit history with hg histedit
Use upload.py -t "Description" --rev rev1:rev2 to upload series of commits as a patchsets for review
After review, commit changes related to different commits to different commits and fold it then using hg histedit
- Repeat previous 2 items until patchset perfection is finished
Perform hg pull and hg rebase in case some commits have been pushed in blessed repo
hg push
Regular work
Generally, there are two main paradigms of DVCS-based community-aware developing: active usage of branches and active usage of history rewriting. The latter gives ability to provide clean, atomic patches which are easy to review, apply and reuse (in other branches and even other projects).
Basically, history-rewriting based developing involves the following issues:
- Try to commit as early as possible and as frequent as possible to maintain independent and atomic commits. It gives ability to work with them easier later.
If you noticed some minor mistake just after commit. you can provide it as additional commit or do hg rollback if it is possible
- This is most important when working on multiple chained features, because usually later it's lot more difficult to split big changeset in several smaller due to changes made one near another.
Good practices: review, what changed (hg status -q, hg diff | less) and commit it (hg commit file1 fileN -m "Commit message" for easy cases — usually this should be enough, otherwise commit more frequently — and hg record (record hg extension) interactive command when you need to split one big diff into several commits — be aware that it does not provide ability to split or edit hunks like git add -i does)
Example: have written some small feature — check that it doesn't break everything instantly (trying to run moin instance and running pytest) and commit it. Even if it breaks and you noticed it too late, fix it and make new commit (or hg rollback; hg commit if you haven't committed anything else yet). Have written some tests — make separate commit. And so on.
Once the work looks done and ready for review, it's time to make commit history prettier. The main tool here is hg histedit (histedit hg extension). It takes last commit id before commits which should be reworked as parameter and provides editor with list of commits where you can set what you would like to do with them.
Note that you should have clean working directory for histedit to work. You can use hg revert to revert files to state in local repository. Don't worry — hg saves backup of changed files with .orig extension by default.
Example. Pretend that we have done about 10 commits and finished with commit 5678. Then we say hg histedit 5668 and given the following file to edit:
pick ab0bc2441518 5669 storage/backends/hg.py: improvements related tag handling pick 3ae216a66dd8 5670 storage/backends: additional methods for hg backend pick e1a4caec2513 5671 storage/backends/hg.py: proper handling of merges pick de8ebe586126 5672 [minor] storage/backends/hg.py: little fix for tags pick 3315eb3e4012 5673 [minor] storage/backends: more metods pick 8e5559a0a08b 5674 storage/backends/hg.py: continuing work on merges pick a89531afebe8 5675 storage/backends/hg.py: branching support pick be11f1e64e73 5676 storage/backends/hg.py: metdata retrieval pick 570e430cd08c 5677 tests/test_hg_backend.py: tests for hg backend pick 9acfab97e561 5678 storage/backends/hg.py: some fixes for branching re # Edit history between ab0bc2441518 and 9acfab97e561 # # Commands: # p, pick = use commit # e, edit = use commit, but stop for amending # f, fold = use commit, but fold into previous commit (combines N and N-1) # d, drop = remove commit from history # m, mess = edit message without changing commit content #
Here you can see some fixups, not very verbose wording, typos and stuff. Now, let's edit file like this:
pick 3ae216a66dd8 5670 storage/backends: additional methods for hg backend fold 3315eb3e4012 5673 [minor] storage/backends: more metods pick ab0bc2441518 5669 storage/backends/hg.py: improvements related tag handling fold de8ebe586126 5672 [minor] storage/backends/hg.py: little fix for tags pick e1a4caec2513 5671 storage/backends/hg.py: proper handling of merges fold 8e5559a0a08b 5674 storage/backends/hg.py: continuing work on merges pick a89531afebe8 5675 storage/backends/hg.py: branching support fold 9acfab97e561 5678 storage/backends/hg.py: some fixes for branching re mess be11f1e64e73 5676 storage/backends/hg.py: metdata retrieval pick 570e430cd08c 5677 tests/test_hg_backend.py: tests for hg backend # Edit history between ab0bc2441518 and 9acfab97e561 # # Commands: # p, pick = use commit # e, edit = use commit, but stop for amending # f, fold = use commit, but fold into previous commit (combines N and N-1) # d, drop = remove commit from history # m, mess = edit message without changing commit content #
- Save and exit. Then histedit begins doing its work on prettifying your commit history. On folds, rewordings and edits it provides ability to change commit message so you can add information about tickets closed/affected, check spelling, provide proper/verbose description and stuff (that means, you can omit this during hacking and concentrate on your task). Sometimes conflicts created as a result of changing of commit order may appear, but usually it is rather easy to resolve them. After it finishes, you'll get new shiny commit history which you can use for reviewing.
Reviewing. In case of utilising codereview.appspot.com, it is possible to use upload.py CLI script provided by this service. One of handy features is its ability to get rev range (via --rev option) as source of patchset.
Example. Pretend that after histedit we have the following history:
4a982b04461d 5669 storage/backends: additional methods for hg backend 8b3c4b85921b 5670 storage/backends/hg.py: improvements related tag handling 97e36f417f6e 5671 storage/backends/hg.py: proper handling of merges 89fbca5396a1 5672 storage/backends/hg.py: branching support 3d997b1235fe 5673 storage/backends/hg.py: metadata retrieval 0e1b4214318f 5674 tests/test_hg_backend.py: tests for hg backend
in this case, we can decide to provide three patchsets for review: regarding generic backend changes, regarding improvements int tag handling and regarding new features. In this case, we say something like this:
$ upload.py -t 'generic backend changes regarding hg backend support' --rev 5668:5669 $ upload.py -t 'hg backend tag support improvements' --rev 5669:5670 $ upload.py -t 'hg backend: new features' --rev 5670:5674
on each run, upload.py provides review ID which should later provided on patchset updates (like upload.py -i <review ID> -t 'Update regarding first review iteration' --rev 5668:5669).
- After another iteration of review it is needed to do changes. The recommended way is to make changes regarding different commits in different commits, then just fold them via histedit and use upload.py again.
Once review successfully finished, changes can be pushed to blessed repo. But in case blessed repo has some changes not presented in local repo, some additional work regarding mergin changes should be done. It can be hg pull && hg merge or hg pull && hg rebase, and latter is more preferred because it maintains linearity of repository, do not garbage it with merge commits and makes life easier.