Gerrymander tips & tricks – viewing comments on a gerrit review without bots
Those who saw my original announcement of gerrymander will know that OpenStack uses the Gerrit tool for code review. One of the efforts made by the Nova (Compute) team during the last release cycle was to increase the automated testing coverage for hypervisors supported by Nova. Previously we only tested Libvirt on KVM, but now there is testing for VMWare, HyperV and XenAPI too. This is great for our code quality, but far less great for the usability of the gerrit web interface. Consider a fairly typical change I needed to look at today, the web UI for gerrit shows many comments. When doing code review of new patch versions though, I mostly don’t care about comments from the “robot” accounts doing automated testing – just the pass / fail status. I’m far more interested in comments made by humans, so I can see whether they’ve been addressed by new patch versions, or whether I agree/disagree with the comments. The gerrit web UI though doesn’t provide any way to separate this info – it just shows a huge list with human & robot comments combined. The ever growing number of robots used by OpenStack for testing mean the human comments are drowned out in the noise
This problem is one of the reasons I created the gerrymander tool. The ‘comments’ command has the ability to display all comments on a change, while filtering out those made by known bot accounts. Now there’s a little setup required, by which I mean you must edit $HOME/.gerrymander to list all the bot accounts. For those using OpenStack though, I’ve uploaded a sample config file, which lists all bot accounts, to the wiki. With this config file in effect I can now look at this same change number 87329
# gerrymander comments --color 87329 Change https://review.openstack.org/87329 (I06c2d9930e0f36a0d7057b6a0f5c9c591caac43f) libvirt: Use os_command_line when kernel_id is set Patch 1 (c8d1cf4559174777be4b42b68379cf78ccd8f382) Jay Pipes: (jaypipes) nova/virt/libvirt/driver.py:3203 You can condense lines 3202-3203 to: guest.os_cmdline = img_props.get('os_command_line') Jay Pipes: (jaypipes) Patch Set 1: I would prefer that you didn't merge this (1 inline comment) Suggestion for consolidation inline, otherwise, looks good. Vladan Popovic: (vladan) Patch Set 1: Thanks for the review Jay. Patch 2 (58323a7271e0f09689b94bc8632204c179934add) Daniel Berrange: (berrange) /COMMIT_MSG:18 "AMI images" is a pretty obscure/non-obvious term. This is about images with an explicit boot kernel set, so please say that explicitly and void the term "AMI" Daniel Berrange: (berrange) Patch Set 2: I would prefer that you didn't merge this (1 inline comment) Jay Pipes: (jaypipes) Patch Set 2: recheck bug 1307344 Patch 3 (8f3ced280d80eaee69029f4bf2a193bcba749284) garyk: (garyk) nova/tests/virt/libvirt/test_libvirt.py:1961 please use self.assertIsNone Mohammed Naser: (mnaser) Patch Set 3: Looks good to me, but someone else must approve garyk: (garyk) Patch Set 3: I would prefer that you didn't merge this (1 inline comment) code looks good. one minor nit Patch 4 (5698a6c7c5195bd1d3ecf01af6fb65c19ae8a990) No comments Patch 5 (a87773776a0d39aa5591fde6caa65b92f5b17a6d) Jay Pipes: (jaypipes) Patch Set 5: Looks good to me, but someone else must approve ++, thx Vladlan! Mohammed Naser: (mnaser) Patch Set 5: Looks good to me, but someone else must approve recheck bug 1307344 Daniel Berrange: (berrange) Patch Set 5: I would prefer that you didn't merge this Jenkins failures are genuine bugs Sreeram Yerrapragada: (syerrapragada) Patch Set 5: recheck-vmware Patch 6 (a1eb12f0c8281c0b01dde00b8225d742b8e832e3) garyk: (garyk) Patch Set 6: Code-Review+1
The obvious limitation here is that the file comments are not shown inline with the code, but overall I think this is still much more useful than the gerrit web UI display of comments. I can still go look at the web UI if I do need to see the context of certain inline comments. So gerrymander comments report complements the web UI nicely.