[asterisk-dev] [policy] Bug Tracker Workflow Discussion

[Russell Bryant]

Leif Madsen wrote:
> BUG TRACKER WORKFLOW
> 
> The workflow in the bug tracker should be handled in the following way:
> 
> 1) A bug is reported and is automatically placed in the 'new' status.

And at this point, they should be closed, since Asterisk has no bugs.  ;)

> 2) The Bug Marshall team should go through bugs in the 'new' status to 
> determine whether the report is valid (not a duplicate, hasn't already 
> been fixed, not a Digium tech support issue, etc.).  Invalid reports 
> should be set to 'closed' with the appropriate resolution.  Categories 
> and descriptions should be corrected at this point.[Note1]

Sounds good.

> 3) The next step is to determine whether the report is about a bug or a 
> submission of a new feature:
> 
>       a) BUG: A bug should be moved into the status 'acknowledged'. The 
> bug may also be assigned to a developer for the creation of the initial 
> patch, or review of the issue. Once a patch has been created for the 
> issue and attached, the issue can then be moved to the 'confirmed' 
> status. At this point initial code review and discussion about the patch 
> will take place. Once an adequate amount of support for the 
> implementation of the patch is acquired, then the bug can be moved to 
> the 'ready for testing' status for wider testing by the community. After 
> the testing phase is complete and it appears the issue is resolved, the 
> patch can be committed by a developer and closed.

This sounds good, as long as it is understood that this is the "full" 
workflow.  For example, if I see a bug in "acknowledged" that I know the 
exact fix for, I would just commit it and close the issue immediately. 
I'd rather not force myself to go through these other steps unless we 
feel it is necessary.  For code written by commiters, it's generally 
easier to go with post-commit peer-review of the patch.

>       b) FEATURE: As new features should be filed with a patch, it can 
> be immediately moved to the 'confirmed' status, making it ready for 
> basic formatting and code review. From there any changes to style or 
> feel of the patch based on feedback from the community can be discussed, 
> and changes to the patch made. It can then be moved forward to the 
> 'ready for testing' status. Once the feature has been merged, or a 
> decision has been made that it will not be merged, the issue should be 
> taken to 'closed' with the appropriate resolution.[Note2]

Sounds good.

> 4) If at any point in the workflow, an issue requires feedback from the 
> original poster of the issue, the status should be changed to 
> 'feedback'.  Once the required information has been provided, it should 
> be placed back in the appropriate point of the workflow.

I really wish we had a way so that the reporter could move it out of 
feedback, but we'll have to put this on our mantis wish-list, I guess. 
It may be worth noting that reviewing issues in feedback for updates is 
another part of the bug marshal team duties.

> 5) If at any point in the workflow, a developer or bug marshal would 
> like to take responsibility for doing the work that is necessary to 
> progress an issue, the status can be changed to 'assigned'.  Once these 
> changes have been completed, it can be placed back in the appropriate 
> place in the workflow.

Sounds good.

> WORKFLOW SUMMARY
> 
> NEW: This issue is awaiting review by bug marshals.  Categorization and 
> summaries should be fixed as appropriate.
> 
> FEEDBACK: This issue requires feedback from the poster of the issue 
> before any additional progress in the workflow can be made. This may 
> include providing additional debugging information, or a backtrace with 
> DONT_OPTIMIZE enabled, for example.
> 
> 
> ACKNOWLEGED: This is a submitted bug which has no patch associated with 
> it, but appears to be a valid bug based on the description.
> 
> 
> CONFIRMED: The patch associated with this issue requires initial 
> formatting and code review, and may have some initial testing done. It 
> is waiting for a developer to confirm the patch will no longer need 
> large changes made to it, and is ready for wider testing from the 
> community. This stage is used for discussing the feel and style of a 
> patch, in addition to the coding style utilized.
> 
> 
> READY FOR TESTING: This is an issue which has a patch that is waiting 
> for testing feedback from the community after it has been deemed to no 
> longer need larger changes.
> 
> ASSIGNED: A developer or bug marshal has taken responsibility for taking 
> the necessary steps to move forward in the workflow.  Once the issue is 
> ready to be reviewed and feedback provided, it should be placed back 
> into the appropriate place of the workflow.
> 
> RESOLVED: A resolution for this issue has been reached.  This issue 
> should immediately be CLOSED.
> 
> CLOSED: No further action is necessary for this issue.
> 
> PENDING RELEASE BRANCH: This status is no longer used. It is a relic of 
> the Asterisk 1.2 days.

These sound good.

> Notes:
> 
> 
> 1) Using the "New / Uncommented" filter is useful for finding these 
> issues quickly.
> 
> 2) The bug tracker now has the ability to monitor the commits list, and 
> if the commit message contains something like, "closes issue 9999", the 
> bug will be automatically closed.

To close an issue, it _must_ be "closes issue #9999".  Note the missing 
#.  Otherwise, to just have the commit noted in the issue, you can say 
"issue #9999", or just "#9999".  (Some other patterns are matched, but 
we can leave the guide at that for simplicity.)

> RELEASE BRANCH COMMIT POLICY
> 
> The code in the release branches should be changed as little as 
> possible.  The only time the release branches will be changed is to fix 
> a bug.  New features will never be included in the release branch unless 
> a special exception is made by the release branch maintainers.
> 
> Sometimes it is difficult to determine whether a patch is considered to 
> fix a bug or if it is a new feature.  Patches that are considered code 
> cleanup, or to improve performance, are NOT to be included in the 
> release branches.  Performance issues will only be considered for the 
> release branch if they are considered significant, and should be 
> approved by the maintainers.
> 
> If there is ever a question about what should be included in the release 
> branch, the maintainers should be allowed to make the decision.

Two things here.  First, please change "RELEASE BRANCH COMMIT POLICY" to 
"1.4 RELEASE BRANCH COMMIT POLICY".  Asterisk 1.6 is managed 
differently.  Also, you can either replace "maintainers" with my name, 
or just note that currently, the person to contact is me.

I'll take it on as my action item to create a separate document that 
describes the current state of all Asterisk releases, their maintenance 
levels, and the commit policies associated with them.  It's something we 
have needed for a while, anyway.

-- 
Russell Bryant
Senior Software Engineer
Open Source Team Lead
Digium, Inc.