[Tracker-discuss] Feature/Change request handling procedure

[Brett Cannon]

On 11/30/06, Stefan Seefeld <seefeld at sympatico.ca> wrote:
>
> Martin v. Löwis wrote:
> > Stefan Seefeld schrieb:
> >> How would you tag patches (attachments, generally) to decide / indicate
> >> whether it actually contains a patch and not some other content, and
> whether
> >> it really fixes the bug ? Doesn't this involve at least some developer
> >> interaction anyway, at which point we could simply set a 'contains fix'
> >> flag that would be part of the bug type, or absorb into the status
> >> enumeration as 'fix_provided_but_needs_testing', as suggested earlier ?
> >
> > I have no preference here. Having a "patch" tag might work; classifying
> > attachments as patches might also work. Typically, *all* patches in
> > "open" submissions still need review: if they are successfully reviewed,
> > the patch gets either accepted or rejected; if accepted, it gets
> > committed and the issue closed. So I don't see the need to distinguish
> > between "has patch" and "has working patch". If the patch is not
> > acceptable as-is, the submitter may be asked to rework it. I had
> > no need to filter for that status: if I open the issue, I can easily
> > see that it is still in the feedback phase (and reject the patch
> > if the submitter is unresponsive).
>
> Martin,
>
> sorry I dropped the ball here. I'm not sure I completely understand, so
> let
> me suggest two concrete procedures. May be that will help us move forward:
>
> 1) additional status enumeration
>
> new -> open -> needs_review -> needs_feedback -> closed
>
> * An assignee opens the issue, finds it has a patch attached, and puts the
> issue
>   into the 'needs_review' state.
>
> * After a successfull review, the patch gets applied, and the issue is
> closed.
>
> * Alternatively, the patch is rejected, so the issue goes into either
>   'needs_feedback' if the submitter is asked to rework the patch, or into
>   'closed' (with resolution set to 'invalid', 'rejected', or some such).
>
> * The 'needs_feedback' would be the pending state we have been talking
> about,
>   with an associated timer to make sure the issue can be closed if the
> submitter
>   is unresponsive.
>
> Of course, the status may also change from 'open' to 'needs_feedback'
> or 'closed' directly, if either there is no attachment at all, or it is
> reviewed quickly.


The only issue I have with this is that needs_review is rather generic.
Does a bug need reviewing to verify it is really a bug?  review_path might
work better.

2) a boolean 'contains patch' flag
>
> The issue would have a 'contains patch' flag that can either be set by the
> submitter, or by an assignee after he verified that an attachment is in
> fact
> a patch. As Paul pointed out, it may be simpler to not expose this flag to
> submitters at all, since, if used the mail gateway, the flag would be
> (almost)
> unaccessible anyway.
> This version doesn't cover the handling of the 'needs feedback' status
> yet.


I really don't think the email worry will be an issue.  Creating issues by
email probably will not happen that often (do we even want to allow it
because of possible spam issues?).  And if we do go with a flag, it should
be controllable by the submitter since it is not that critical of a thing
for us to control.

In terms of schema / workflow complexity, the two are roughly equivalent,
> though the first is a bit more expressive. (In the second scenario, can
> the
> 'contains patch' flag be unset ? How does the assignee mark that the patch
> needs
> to be reworked ? Etc.)


At the moment comments carry the brunt of the burden to signal that a patch
needs work.  It's worked out, but then again if we have another status
specifying that a patch needs review then perhaps we can eventually use that
to send out reminder emails to tend to it and then auto-close eventually.

-Brett
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.python.org/pipermail/tracker-discuss/attachments/20061130/eae3e110/attachment.htm