Thursday, November 8, 2007

Thunderbird Bug Fixing Lab - an up close and personal lesson on patch review

Filing a bug

First I had to create a bug over on landfill - explaining the issue

The gist is that anything that has characters@...Someothercharacters turns into a mailto: link in Thunderbird and this was annoying Dave so the whole class jumped to fix it.

Fixing the problem

Dowloaded and built a copy of Thunderbird's trunk on my MacBook and then navigated over to mozTXTToHTMLConv.cpp in order to alter some code which checks for a '.' after the @ symbol but does not also check that there is not a ".." in that string.

This is a one-line fix but I was having some trouble getting my code changes to show up. Funny story, this happened to me in class too and both times it was because I forgot a ) in the code.

Creating a patch

This is easily done by calling cvs diff -u8p . > mailtoPatch.txt in my mozilla directory.

Requesting Review and the Results

We were going down the line as reviewers - I asked Mmullin and he had already done a review so he handed me off to Armenzg who passed my patch with the comments to remove my printf statement and to make the two if statements into one evaluation. It turns out that he should not have passed my patch, instead he should have made the comments and let me fix it before approving the patch.

Even though Armenzg passed my first patch, I did in fact re-do the patch minus printf and simplifying the if statement. I am waiting for a proper approval as I write this but I am sure it will be approved.

After it was all over, I was then the reviewer for Peter's patch and I was impressed with his one line change and passed it immediately.

The whole process was a great learning curve. Now that I've seen how the patch/review process works I imagine that I could fix a bug one of these days.

No comments: