[wp-hackers] Recommendations for trac patches?

[Andrew Nacin]

Answers to some of your comments:


> To me, the has-patch keyword denotes that I, or the person who submitted
> the patch, has tested the patch against the build cited inside the diff and
> believes that the patch should be added to core. If the patch submitter did
> not believe this, why would they submit the patch?


In all honesty, many patches are not tested on first upload. (Or, at most,
it may be executed to ensure PHP parses it, more or less a typo sanity
check.) Many can be written by running the code through one's head.
Additionally, "tested" is not simply that the patch parses, or even
works.  It implies a far more rigorous standard. There are various
dependencies, edge cases, unit tests, etc., that often need to be
considered.  Plenty of patches are uploaded that "work" but don't belong
near core in their current state, but that's okay, because they're workable.

I've found the use of the commit keyword can be boiled down to I suppose
three general situations. One, patches that are very easy commits -- say,
the obvious typo, faulty logic, or inline documentation.

Two, once a patch has been tested, a core contributor or developer may add
the keyword when they feel it is ready for inclusion. Generally the
ramifications of the patch (will it break anything? does it introduce any
new considerations?) at this point are well understood, and it deserves
inclusion (has there been traction?). And there is a difference between
being ready for inclusion and deserving inclusion.

The third situation comes down to deserving of inclusion -- the patch is
ready to go into core (see #1 and #2) but needs some review from a core
developer, who would then either commit it, or close the ticket as
worksforme (i.e. plugin material) or wontfix. Punting or other wrangling are
options as well.

A guide that details what the keywords are, when to use specific keywords,
> how to select the proper component, etc would be greatly useful. If a solid
> example of what the core team desires in a patch ticket is provided, that
> would be even better.


dd32 proposed a series of suggestions about a month ago. It would make sense
that a better guide is put together, yes. What I wrote here about keyword
use is only what I have picked up on -- it is not documented anywhere
really, and if you ask someone else, you may get different answers.

As a final note, I don't want to start CCing different devs just to get
> attention.


You can't, actually -- you would need to have their e-mail address in your
own profile. (Entering your username in the CC field will add you to the
ticket and I suppose indicate your interest, but won't generate any
e-mails.) The most you could do is assign the ticket to one of them, which
usually just happens with default component owners anyway. IRC is definitely
a good avenue for discussing patches. On occasion, I might even attach a
patch to a ticket without comment, then bring it up in #wordpress-dev.