Missing the normal code review process

My favorite Bonker's World cartoon points out that the code review process alone does not necessarily ensure the quality of the code.

When you are living in the open, however, you as a reviewee are restrained with an extra incentive to be more careful, lest you embarrass yourself in public with silly mistakes. The reviewers have the same extra incentive to find flaws in others patches to earn their karma points. It also helps that any bug becomes shallow because the reviewers competing to find issues in your code gives any change a large enough number of eyeballs.

It is no wonder that the open source process can produce better-quality code more easily.
Unless you are working on fixing a vulnerability that is still under embargo, that is. It is even worse if the bug is in an obscure corner of the system you do not personally use very much.

You can only confide in a few of your trusted lieutenants, who all tend to think in a way similar to yourself. This greatly increases the chances of simple and silly bugs go unnoticed. And it makes you feel uneasy and stressed. You get only one chance to get this right, or the disclosure goes out. This is especially true when you thought you have everything tagged and ready, and decided to take a nap while waiting for the embargo to expire. It suddenly occurs to you that there was a corner case you missed, and you have to scramble to redo the fix.

Now that the embargo is over and the release is out, I can relax ;-)

Cycle 1.7.7 begins

During the first week since 1.7.6 release...

• Merged topics that have been cooking in the next branch for a while to the master branch to release them as part of 1.7.7 release;
• Updated the Git Calendar for the next cycle.

As I ended up working on some topics around large-ish objects during the last cycle, I may continue to do so during this cycle as well. I am currently looking at a few aspects of pack-objects and fsck.

Queued 19 patches from 8 people.
Made 18 merges to 'master' branch, to include in the next release.
Made 35 merges to 'next' branch for public testing.
Made 2 merges to 'maint' branch, to include in the maintenance release.

Cycle 1.7.6, day #3 of week #9

Previous reviews resulted in a few topics to be re-rolled and/or rebased.
With Shawn, met David Barr who was in town.

Earlier I was planning to skip 1.7.6-rc3 and go straight to 1.7.6 final, but decided to do a final round anyway.

Sent out the fourth issue of "What's cooking" summary for the month.

For the first time, FC15 test bochs managed to produce the set of release RPMs (I had missing build-dependencies here and there but this has never been a high priority for me, so...).


Commented on 6 topics on the list.
Tagged v1.7.6-rc3.
Queued 28 patches from 6 people.

Cycle 1.7.6, day #2 of week #9

Overnight, the publisher of my Git book notified that they found a misprint in the latest print run. Prepared material to update the affected readership.

Ram's update to "revert/cherry-pick" refactoring is getting into a reasonable shape, to be ready for the next cycle. I am reasonably happy with it.

Commented on 12 topics on the list.
Queued 14 patches from 2 people.

Cycle 1.7.6, day #1 of week #9

A handful more topics, most notably an update for the external transport helper interface, are being reviewed and discussed.

Commented on 8 topics on the list.
Queued 8 patches from 5 people.
Merged 1 topic to 'master' branch, to include in the next release.

Cycle 1.7.6, week #8

Mostly an uneventful week. Tagged 1.7.6-rc2 and without any serious regression report, it seems that we are good to go to the final release without -rc3.

Topics that are meant for the post 1.7.6 cycle are still flowing in. Notable are to enable server side compression for git-archive remote access interface, and a re-roll of transport-helper fixes.

Tagged 1 release.
Queued 45 patches from 12 people.
Made 2 merges to 'master' branch, to include in the next release.

Cycle 1.7.6, day #6 of week #7

Finally found time to start reviewing a large merge-recursive patch series from Elijah, but ended up getting distracted by many other chores.

Updated "What's cooking" periodical and sent it out. Didn't do any coding today.

Queued 62 patches from 2 people.

Cycle 1.7.6, day #5 of week #7

Realized that we use zlib API incorrectly throughout the system; not just we assume we can set up the stream with input or output of more than 4GB, we also assume that we can pass the z_stream object around and expect it to keep track of the progress, which is definitely not true if our buffer is larger than what can be expressed with avail_in/avail_out (limited to 4GB).

Scrapped the approach of yesterday's patch series, and tackled the issue with a different approach, namely, to write a thin wrapper layer on top of zlib API to pretend that the API can accept buffers that are larger than zlib supports by making separate and repeated calls into the underlying inflate/deflate as necessary. The trickiest part was not to give Z_FINISH to earlier rounds of inflate/deflate when the buffer is large and the wrapper is feeding the underlying library in separate rounds, which was a stupid bug in my earlier attempts that took quite a long time for me to figure out what is going wrong.

Sent it out as a 7-patch series, and queued them myself.

Cycle 1.7.6, day #4 of week #7

Was sick most of the afternoon, so not much done, except for:

• found many misuse of zlib API in our code that assumed we can feed more than 4GB at once to inflate/deflate, which was not a case;
• fixed a few instances and sent patches to the mailing list;
• commenting on a few topics on the list.

Queued 4 patches from 2 people.

Cycle 1.7.6, day #3 of week #7

Jeff King was in town and together with Shawn we had lunch and exchanged gossip about many other git people behind their back (no, we actually didn't badmouth any of them).

Noticed that our call to zlib, when we feed a large blob to it from memory in one go, was totally busted. Sent out a fix for one instance, hoping that others may find a better way to address this issue by coming up with an easy to use wrapper and then fixing all instances of such incorrect uses using it, including this one.

Spent way too much time reading and regurgitating recent list discussions and patches that came after -rc0, and forgot that I was supposed to tag -rc1 today. Starting the integration cycle late in the afternoon is never a good idea.

In any case, updated the draft release notes, and pushed out 1.7.6-rc1. I am hoping that we won't need -rc3 and can release the final in two weeks. Knock wood...

Commented on 11 topics.
Tagged v1.7.6-rc1.
Queued 7 patches from 4 people.

Cycle 1.7.6, day #2 of week #7

The topics that are cooking for the next cycle are already making good progress. Among reviewed topics, the namespaced-ref topic is quite promising in that it would help constructing cheaper and safer hosting sites. An updated document that fills the gap of description in the pack transfer protocol related to the "shallow clone" is almost there, and presumably compatible implementations can now be written using the specification.

Commented on 10 topics.
Queued 17 patches from 4 people.
Merged 1 topic to 'master' branch, to include in the next release.
Merged 1 topic to 'next' branch for public testing.

Cycle 1.7.6, day #1 of week #7

Almost ready to tag 1.7.6-rc1; merged the last batch of safe fixes and documentation updates to the "master" branch.

Seems that "submodule add ../relative/path" discussion has come to a conclusion people are reasonably happy with. Matthew's students also are doing an interesting work writing importer from MediaWiki. It's good to see people doing good work without getting too distracted with noise.

Commented on 7 topics.

Queued 7 patches from 3 people.

Merged 4 topics to 'master' branch, to include in the next release.

Merged 5 topics to 'next' branch for public testing.

Cycle 1.7.6, day #7 of week #6

Fixed a funny bug that dates at least back to v1.7.3 days. Trying to create a branch with "checkout -b" with a name that is the same as the output from "describe" (which by the way is not a recommended practice) did not work, even though giving the same name to "branch" allowed the named branch to be created.

Sent out the regular "What's cooking" summary. As we are post -rc0, things should calm down a bit for coming couple of weeks.

Commented on 5 topics.
Queued 18 patches from 6 people.
Merged 2 topics to 'next' branch for public testing.

Cycle 1.7.6, day #6 of week #6

Day off.

Cycle 1.7.6, day #5 of week #6

Started to revisit a stalled topic to get rid of inefficient verify-pack that walks the objects in a pack in their object name order (which breaks the locality in the pack completely). The index-pack code has to internally perform the same task, and it does take object locality into account, so there is no reason not to take advantage of it and share the code. This will in the future help us to maintain one single code for packfile verification, which needs to be tweaked when I introduce a narrow clone properly. One of the important things pack verifier needs to be aware of is that it will no longer be an error if objects that are referenced by an object in the pack do not exist in the repository (when they are outside the narrowed domain).
Sent a four-patch series for review to the list.

Commented on a dozen topics.
Queued 2 patches from 2 people.

Cycle 1.7.6, day #4 of week #6

Somebody found an undocumented option to "status" that I didn't even know existed, and it did not work as advertised in a corner case. Peff quickly diagnosed and fixed it correctly.

Commented on 7 topics.
Queued 5 patches from 4 people.
Merged 3 topics to 'next' branch for public testing.

Cycle 1.7.6, day #3 of week #6

Tagged the "preview" release for 1.7.6. There are many smallish new features and usability enhancements in isolated commands (e.g. "git merge" can be told to merge the "previous" branch with "-"), some bugfixes (most are back-merged to 1.7.5.X series), and a few changes that affect the user experience globally (e.g. the magic pathspec ":/" that tells operations to work on the full working tree).

Tagged v1.7.5.4.
Tagged v1.7.6-rc0.
Queued 2 patches from 2 people.
Merged 1 topic to 'master' branch, to include in the next release.
Merged 6 topics to 'next' branch for public testing.
Merged 5 topics to 'maint' branch, to include in the maintenance release.

Cycle 1.7.6, day #2 of week #6

Gave the final round of eyeballing to the list of topics that are still cooking, to categorize them into the ones to include in the upcoming release, and the ones to defer. Decided to postpone the streaming checkout topic to the next cycle, but did some benchmark to make sure it is sound.

Commented on 6 topics.
Queued 6 patches from 3 people.
Merged 12 topics to 'master' branch, to include in the next release.
Merged 6 topics to 'maint' branch, to include in the maintenance release.

Cycle 1.7.6, day #1 of week #6

Holiday but ended up working a bit on Git, some reading of list messages and a lot of re-reviewing of the list of topics in cooking to decide which ones to include in the upcoming release.
Commented on 7 topics.
Queued 3 patches from 3 people.
Merged 3 topics to 'master' branch, to include in the next release.
Merged 11 topics to 'next' branch for public testing.
Merged 3 topics to 'maint' branch, to include in the maintenance release.

Cycle 1.7.6, day #7 of week #5

Again, not much done, except for scanning the list traffic where I found nothing urgent. It is a long weekend in the US and I guess everybody is taking things slowly. I can afford to live slowly for a change, too.

Finished the initial machine tweaking of the FC15 bochs well enough to build and test "next" branch of git successfully (I am skipping t9xxx series but I have been skipping them on FC13 bochs for some time, so this is not a regression).

Drove to the airport and fetched my wife in the afternoon, and then helped her unpack.