Bug 198271 - Auto-Assigner: IF attachment "is patch" THEN set maintainer-approval flag value (to + or ?) based on <condition>
Summary: Auto-Assigner: IF attachment "is patch" THEN set maintainer-approval flag val...
Status: Open
Alias: None
Product: Services
Classification: Unclassified
Component: Bug Tracker (show other bugs)
Version: unspecified
Hardware: Any Any
: --- Affects Only Me
Assignee: Mahdi Mokhtari
URL: https://lists.freebsd.org/pipermail/f...
Keywords: dogfood, easy, feature, needs-patch
: 206974 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-03-04 09:09 UTC by Kubilay Kocak
Modified: 2018-05-23 10:27 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Kubilay Kocak freebsd_committer freebsd_triage 2015-03-04 09:09:42 UTC
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.
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2015-11-02 08:29:04 UTC
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.

Note 3:
Comment 2 Kubilay Kocak freebsd_committer freebsd_triage 2015-11-02 08:32:10 UTC
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
Comment 3 Marcus von Appen freebsd_committer freebsd_triage 2015-11-03 13:20:57 UTC
(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.
Comment 4 Kubilay Kocak freebsd_committer freebsd_triage 2015-11-03 15:08:10 UTC
Right, so any attachment without a flag set effectively ends up meaning no maintainer anyway. Sounds good
Comment 5 Kubilay Kocak freebsd_committer freebsd_triage 2015-12-30 11:05:52 UTC
This is a dogfood issue as it improves the productivity of the triage process and human resources
Comment 6 Kubilay Kocak freebsd_committer freebsd_triage 2016-02-06 13:17:45 UTC
*** Bug 206974 has been marked as a duplicate of this bug. ***
Comment 7 Eitan Adler freebsd_committer freebsd_triage 2018-05-23 10:27:51 UTC
batch change of PRs untouched in 2018 marked "in progress" back to open.