[Adium-devl] Trac "patch status" field

[Colin Barrett]

So after using this to do some triage, I think we can pair this down a  
bit.

Workflow as I'm seeing it:

When a patch author puts a patch up, they set the status as Needs  
Review.

When a developer comes along, if they are sure the patch is, as it is  
right now, no good, they mark as Rejected, and give a reason why. A  
patch author can then go back to the start and submit a new patch and  
set the flag to Needs Review.
If they are unsure about the patch, they leave it as Needs Review.  
Patches should not stay in this state for very long. It should be  
quickly determined if that particular patch is good or not good to be  
checked in.
Once the patch as it exists on the bug is OK to check in, the  
developer should mark it as Accepted, and then check in the patch,  
closing the ticket.

This is similar to how the r? r+ r- system works on Mozilla, and it's  
pretty efficient. The only thing is that in Bugzilla (the bug tracker  
for Mozilla), we can set the r flag on individual attachments (which  
would be quite nice here, as well, if anyone wants to hack on  
trac...). Not a big deal if trac doesn't support that.

Anyone have any objections? The main reason I'm proposing this is that  
I think "needs discussion" is a useless state: the ticket properties  
should be used to determine if a feature should be included or not.  
Patch flags should only be talking about the patch. The feature itself  
should be triaged with the existing ticket-based flags.

Thoughts?

-Colin

On Mar 18, 2007, at 7:32 AM, Evan Schoenberg wrote:

> I've added a "Patch" custom field to Trac.  I think this should  
> replace the "Patch included" checkbox. Current values:
> ---
> None
> Initially Included
> Needs Discussion
> Needs Dev Review
> Needs Changes by Author
> OK; Needs Check-In
> Accepted
> ---
> Are there any other values that it should have?  My thinking is that  
> we'll ask patch submitters to set the ticket to "Initially Included"  
> when they attach the patch.  Needs Discussion will be used when it's  
> questionable whether the patch should be included in Adium or not;  
> if it's a bug fix or decidedly a good thing but needs code review,  
> that's what "Needs Dev Review" will be.  I welcome suggestions as to  
> different phrasings that might be more clear - much better to make  
> changes now than later.
>
> Trac doesn't want to let old custom fields die... so there's a  
> duplicate copy of "Patch" from when I had named it  
> "field_patchstatus" before deciding that name was ugly.  I didn't  
> expect it to keep both around... as a temporary work around, I've  
> added the custom field back with a label of "(Remove Me)" and a type  
> of "hidden" -- which is invalid -- so that there aren't two  
> seemingly valid dropdowns.  I'd appreciate it if someone could ask  
> in #trac or ask a Trac developer what we need to do to remove all  
> references to an old custom field.
>
> The only place the "field_patchstatus" field is used is in #6541  
> where I tried it out before changing it to "patch_status".  We'll  
> want to be able to remove the "field_haspatch" checkbox, as well.
>
> Cheers,
> Evan
>
>
> _______________________________________________
> Adium-devl mailing list
> Adium-devl at adiumx.com
> http://adiumx.com/mailman/listinfo/adium-devl_adiumx.com