Bugzilla contains an "Attachment is patch" attribute for issues that contain attachments where type [patch] has been selected (by the reporter).
Currently maintainer-approval flags on attachments are manually (not automatically) created, leaving most issues with "patch" attachments undiscoverable, unless:
a) Issue has a maintainer, where maintainer-feedback is set, OR
b) Triage sets keyword:patch, OR
c) user understands bugzilla advanced search
Issues with solution (a) include:
- not addressing approval versus simple acknowledgement/reply, and
- ambiguity between feedback and approval
- not actually solving the problem of 'has-a-patch' discovery (for interested committers)
This has led to some people using the maintainer-feedback flag to denote patch approval. In particular it also has caused confusion for triage and ongoing issue tracking in a number of scenarios, where:
a) multiple revisions of patches are attached
b) multiple patches by multiple contributors are attached
The more productive to begin the maintainer approval process and ensure that issues of this type are easily discoverable is to classify and annotate issues as completely as possible, and is as follows:
a) If an issue has a maintainer, set maintainer-feedback, AND (this we do)
b) If an issue has an attachment of type [patch], set maintainer-approval, set keyword: patch (this we dont do)
The subsequent workflow from a maintainer perspective may be as follows:
- Acknowledge the issue (maintainer-feedback +), reject the patch (maintainer-approval -)
- Acknowledge the issue (maintainer-feedback +), approve the patch (maintainer-approval +)
- Acknowledge the issue (maintainer-feedback, comment
- Approve the patch (maintainer-approval +), dont touch maintainer-feedback (implicit)
For cases where issue/port does not have a maintainer, it also becomes trivial to search for issues with patches, with keyword:patch (all else being equal), which we set in other circumstances (triage, summary contains [patch])
It also removes the burden from triage resources by correctly classifying issues-with-patches without requiring manual intervention, or users (reporters, triagers or committers) to search for or understand Bugzilla's advanced search scheme.
To summarise comment 0
When an attachment (of type patch) is created, the auto-assigner needs to:
1) Set the attachment maintainer-approval flag:
1a) To value: ? <maintainer-email> if the attachment submitter != maintainer
1b) To value: + if the attachment submitter = maintainer
2) Add the keyword: patch to the issue. This will also make adding [PATCH] to summaries redundant, since this will more directly reflect that an issue actually has a patch.
In addition to (1a) and (1b), consider:
1c) Set flag to value: - if there is no maintainer. A comment should also be added in this case. Alternatively, it could be set to + (to mean implicit approval). I haven't decided which one is more 'correct'. On one hand, a port without a maintainer cant by definition have maintainer-approval, on the other, we want to show that it doesn't need it, or implicitly has it.
Note 1: The "attachment" submitter may be different to the "issue" submitter. The former not the latter should be used for this mechanism.
Note 2: This should trigger on every attachment creation, not just initial bug creation.
CC'd Mat because he's a perl wizard and expressed interest in the areas (that require perl tweaking) that we want to improve in bugzilla
(In reply to Kubilay Kocak from comment #1)
> 1c) Set flag to value: - if there is no maintainer. A comment should also be
> added in this case. Alternatively, it could be set to + (to mean implicit
> approval). I haven't decided which one is more 'correct'. On one hand, a
> port without a maintainer cant by definition have maintainer-approval, on
> the other, we want to show that it doesn't need it, or implicitly has it.
Since there's noone to approve (or reject) it, I'd leave it untouched instead.
The patch can be of whatever quality, related to the issue or not and depending on those circumstances either of the flag values can be wrong.
Right, so any attachment without a flag set effectively ends up meaning no maintainer anyway. Sounds good
This is a dogfood issue as it improves the productivity of the triage process and human resources
*** Bug 206974 has been marked as a duplicate of this bug. ***
batch change of PRs untouched in 2018 marked "in progress" back to open.