Wednesday, October 18, 2006
Code Review Avoiding Patches
I am a big believer in code review , it is what makes Mozilla strong. I am very suspicious when people try to avoid review. Regardless what the excuse is at the end usually the code has errors that proper review would easily have been resolved before check in. This was my main concern with the initial Phoenix Firefox development. Typically I am not a big fan of Mozilla corporate decisions as they are frequently done behind the scenes. I blame this to the bad Carma of Mountain View which is inherited from the old Netscape days.
However there is one thing where I stand 100% behind Mozilla and that is the trademark policy. If you say a thing is a Firefox than it has to be a Firefox and nothing else. If Debian makes modifications and then refuses to get them properly reviewed by people who understand the code the output is predictable: lower quality than the original code base. Mozilla says this lower quality product shall not spoil our name and is IMHO right to do so.
So lets have a walk what that means:
I installed a while ago Ubuntu on my brothers machine to get rid of spyware etc. At this time Mozilla 1.7 was shipped and I was pretty unhappy with the Firefox development cycle (see above). I launched the application and found the layout debugger?!? Ubuntu does not use a typical bugzilla so i was pretty lost but anyway I filed the corresponding bug . So what was the root cause of this bug. Somebody at Debian spoiled it till today ( search for extensions=all or follow bug 393859). Nobody at Mozilla, who has at least a minimal build understanding will approve such a patch. And I think that it would be good for Debian too as it would not ship applications with a broken user interface and dubious security.
This would certainly limit the freedom of Debian to modify the source. But why should users of Debian and many Ubuntu users suffer from bad preventable code changes?
So to whom it applies: if you need help porting a patch that I have written to Debian because you follow the very bad idea of maintaining the Firefox 1.0.x series (its a security problem in my eyes) file a patch in bugzilla and ask me for review.
UPDATE: I just came across the blog of the Firefox maintainer:
- Changes to disable application upgrade (we want that to happen through apt-get) and change some other default preferences,
- Changes to fix “make distclean” so that it really cleans the build directory,
- Change not to build the “mangle” utility,
- Change not to call netstat to generate entropy, which is useless on linux,
- Changes to work around bugs with the hidden visibility pragma on gcc,
- Changes to make the pango backend actually build correctly,
And you don't want that people who wrote code review this???? Has the pango backend even a tinderbox? If mozilla has not shipped the test builds before how can that back end have the proper testing?
