Changes

Jump to: navigation, search

Bugzilla:Review

3,993 bytes added, 20:47, 10 July 2007
no edit summary
In Bugzilla, almost every patch that is submitted goes through a process of review, where it is looked at by an experienced Bugzilla developer to make sure that it meets our standards for code quality.

The review process is ''very important''. Without it, Bugzilla's code would quickly become a terrible mess. (A lot of Bugzilla's code is currently a terrible mess, but that's a result of weaker review processes from Bugzilla's history.)

== How to Ask for Review ==

When you attach your patch, you'll see a "review" flag. You want to set this to "?" and put the username of the correct Bugzilla reviewer in the box next to the "?".

How do you know who's the right reviewer? Use the [http://www.bugzilla.org/docs/reviewer-list.html|List of Reviewers], which explains the whole thing.

== How the Review Process Works ==

If the reviewer thinks your patch is acceptable, he will change the review flag to a "+". If there are things you need to fix, the reviewer will mark it with a "-".

If there are things you need to fix before we can accept the patch, the reviewer will change the review flag to a "-". He/she will include comments about what needs to be fixed.

Sometimes the reviewer will prefix his comments with "Nit:". This means that he's just "nitpicking"--you don't ''have'' to fix these points, but we'd like you to.

If you disagree with a review, just respond to it in the comments! This isn't a dictatorship, it's an open-source community.

=== Who Fixes The Patch? ===

'''The author of the patch''' is always the one who revises the patch if it gets a "-" on its review. The reviewer probably ''could'' fix your patch, but every Bugzilla reviewer is very busy and doesn't have the time or the desire to do so.

Also, it's a better learning experience for ''you'' to fix the patch.

Basically, we look at your patches as your responsibility. Once they are attached, it's up to you to revise them until we accept them.

=== What If I'm Waiting Too Long for My Review? ===

If you've waited longer than a week for your review, come into [[Bugzilla:Communicate|IRC]] and bug us. If you can't get a hold of your reviewer on IRC, just send him or her an email.

If you wait longer than two weeks for your review, pick another reviewer from the [http://www.bugzilla.org/docs/reviewer-list.html|reviewer list]. If there isn't another reviewer, just keep communicating with the current one in IRC or by email until you get a result.

== It's Not a Personal Criticism ==

All of us work very hard on our code, and we all take pride in our work. Sometimes, getting criticized for what we've done is difficult, particularly when we've spent a lot of time on it.

Bugzilla reviewers may seem to be harsh, accidentally. They aren't trying to be harsh or overly critical, they're just pointing out what needs to be changed, which usually means they're pointing out what's wrong with the current code, instead of pointing out what's ''right'' with it. Usually they don't have lots of extra time in their life for reviews, so they just quickly write what needs to be fixed, without spending too much time thinking about the nicest way to say it. Sometimes they also don't go into long explanations.

You're not a terrible programmer or a bad person. All we're doing is telling you what's preventing us from checking the code into Bugzilla's main codebase. It's not even that you're wrong, it's just that there are certain things we need before checking code into CVS. Sometimes those requirements may seem unusually strict to new contributors--trust me, they're necessary, from long experience working on Bugzilla. They may seem hard now, but you'll understand after a while of working on the Project.

And remember, anything we didn't say was wrong, that was good code!

Keep on submitting patches, and you'll get better in time, and reviews will get easier and easier. Even if sometimes it might seem hard, it's a great learning experience, and it really makes Bugzilla a better product.
Canmove, confirm
345
edits

Navigation menu