happy bmo push day!

release tag

the following changes have been pushed to bugzilla.mozilla.org:

  • [1523317] Exclude Graveyard products from QuickSearch results
  • [1512815] Optimize Bugzilla->active_custom_fields() for CPU and memory usage
  • [1524213] phabricator revisions list on bug page has extra / in the revision link
  • [1523404] Cannot clear all scopes when editing an oauth2 client. Throws DB error
  • [1525308] Custom Bug Entry Form for Blocklist Policy Requests
  • [1525451] Update triage owner report defaults
  • [1524158] markdown generated by approval comment form could be improved
  • [1525808] Remove CC changes from activity stream
  • [1476111] Enable syntax highlighting in comment code blocks
  • [1528334] Adding image to main bugzilla screen for User Research
  • [1047539] Bugmails including “See Also” bug links do not include a “Referenced Bugs” section with the summary of the other bug
  • [1402894] Remove “Restrict this session to this IP” option from login page
  • [1461492] Add an optional regressed-by field in bugs
  • [1528277] Add “Has STR” and “Has Regression Range” fields for the ‘External Software Affecting Firefox’ product

discuss these changes on mozilla.tools.bmo.

An experiment for making big changes with smaller commits on GitHub.

It is safe to say that smaller commits is a perennial topic at Mozilla.

On the reviewer side, I definitely prefer smaller commits.
I find it’s easier — and faster — to review them. Often not in the neighborhood of microcommits, but at least 300-400 line patches.

For the thing I work on we use GitHub, and PRs seem to work best on small things. When tasked with a large task, it seems that many engineers will break it up into suitably small PRs and things work well.

However sometimes tasks are more complicated than you’d imagine.

Bug 1437646 seemed simple — let’s just log JSON to standard out. But logging json from apache is hard (for a variety of reasons outside the scope of this post).

At first I was able to split this into two tasks:
1. add a log server that listens on localhost:5880 and serializes all lines sent to it to stdout.
2. (finally) implement Log::Log4perl

However Bugzilla is not simple; it does not yield easily to such changes.
We need a place to keep the log4perl config file(s), and the wrapper I wrote to start apache in docker isn’t smart enough to wait until our socket-logging-server is running. So what was planned as roughly two tasks resulted in two relatively large PRs.

So the problem is — how can I make this easier to review, using the tools available? In the case my own discomfort is an an acceptable trade-off if it means my code can be reviewed more easily.

Below is what I came up with — and it’s immediately useful though far from perfect.

  1. Work out a series of commits (microcommits or at least smaller ones)
  2. for each commit, create a branch. This means for 4 commits you have 4 branches. For the examples below we’ll use feature/1, feature/2, feature/3, and feature/4. Imagine that “feature” is a more descriptive word.
  3. For feature/1, make a PR that would merge it into feature/0-reviewed
  4. Make a branch that is identical to feature/1, called feature/1-reviewed
  5. For feature/2, make a PR that merge it into feature/1-reviewed
  6. Continue this until the end. Every feature/N will merge into feature/N-reviewed, and feature/N-reviewed will be equal to feature/(N -1).

As a result of doing this, there will be 4 PRs that each only show the differences of the commits in question. After they’re all reviewed, it will still be required to cherry-pick each of the reviewed commits into a final branch and make a PR out of that, but getting that (rather larger) PR approved
can be a less involved process than code review — perhaps even the responsibility of a bot?

I’ve got some shell scripts to accomplish this, so I’m going to try it a bit and see what the results are.