291092 - Developer's Guide should describe more specifically what it requires for us to accept a patch

Status: NEW

(Bugzilla :: bugzilla.org, enhancement)

Description

[Max Kanat-Alexander]

Right now the Contributor's Guide tells you how to post a patch, and what the
actual mechanical processes for review are, but it doesn't actually describe the
sorts of patches that we will *accept* or give a positive review to. :-)

Here are a few thoughts for modification of the file:

RULE #1: Don't change any parts of Bugzilla that don't NEED to be changed by the
feature.

RULE #2: Break down your feature into the smallest bugs possible, and fix them
one at a time, in such a fashion that they can be checked-in incrementally.

Comment 1

[Jouni Heikniemi]

And, in practice, a key condition to acceptance of a patch is its readability
and ease of approach. For this reason, I'd like to see the guide say something
like this:

"When requesting for review, write a meaningful description of what the patch
does and how. Explain all the architectural changes and algorithmic parts that
aren't clear by looking at the code (although they usually should be!). You
don't have to repeat for each revision of the patch, but it is useful to recap
the changes between patches."

I've found a good rule of thumb to be "One sentence of description for every 2
kB of the patch", but it naturally varies enormously by patch type and coding
style. Perhaps we shouldn't guide people on the length of the description but
rather the content.

Comment 2

[Chris Raplee]

If it all 'just happens' then it means things are working right.

If it all 'happens the way we expect it to' then we're doing the right thing. 

If 'we need to make the right thing happen' then we're on the wrong track

If 'something is not happening the way we expect' then it mean things are 
working wrong. 

Comment 3

[Liz Henry (:lizzard) (relman/hg->git project)]

Discussion of this is currently on an etherpad linked from bug 908563.