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.
[...] this in google alerts, about one person’s process of fixing a thunderbird bug. It’s nice to [...]
[...] this in google alerts, about one person’s process of fixing a thunderbird bug. It’s nice to [...]
Just a quick note on reviewing: you say that Armenzg should have made the comments but not approved the patch as-is, waiting for a fixed version instead. That’s how it should work initially, when a reviewer isn’t familiar with the reviewee’s prior patches and work. Given enough time, if the reviewer has sufficient trust in the reviewee to make the changes correctly, based on past performance, he can approve a patch “with comments”, under the assumption that the person who made the patch will appropriately update it before having the patch committed.
At this stage you probably fit in the first group, but eventually you might find the second group to be more efficient. In any case it’s perfectly fine to stick to the first group if that’s what you’re comfortable doing.
I have never heard about Landfill before. Isn’t http://bugzilla.mozilla.org the proper place for Thunderbird bugs?