[Patchew-devel] [PATCH v2 00/12] Queue, MAINTAINERS and watched query

[Paolo Bonzini]

On 28/11/18 15:34, Fam Zheng wrote:
>>> What is missing is the isolation between the existing maintainers role that
>>> takes care of the testings etc, and the new maintainer role that is only
>>> interested in receiving above notifications.
>>
>> Why is there a conflict?  I understand that the new functionality is for
>> any logged-in user, including getting email.  As long as there Maybe
>> it's just a naming issue?
> 
> The email/git/testing config things are visible to maintainers, and are too
> much information and too detailed for people who only want to configure and
> receive such reminder emails.

Got it.  Perhaps we should add a per-user setting for "Receive
maintainer notifications" (or even tristate: yes, no, only if not in
recipients) and use it in the reminder email hook.

It shouldn't be hard to add a basic "user settings" form, but it can be
done later.  In the DB this can be a UserProperty table with one-to-many
relation to User.

>> Perhaps one possibility is removing merged patches from the special
>> queues (accept/reject/watch), using a MessageMerged event.  Also adding
>> an "empty queue" button to the "my queues" page.  Otherwise looks great.
> 
> What does the "empty queue" button do?

Remove all patches from the queue.  For example you could use it to
clear the "acked" queue after sending a pull request.

>>> Every user can have a record in WatchedQuery. The query in this record
>>> is going to be run against every new series, and if matched, the patches
>>> will be added to the 'watched' queue, making this the 3rd specially
>>> purposed queue.
>>
>> Why not one-to-many?  (That is a user can have >1 record in
>> WatchedQuery).  This can come in handy when people go on vacation...
> 
> It makes sense, but for now I want to limit it to 1 query per user for a few
> reasons:
> 
> 1) UI is simpler. The added button just overwrites the existing one everytime,
> so there's no watched query list page needed. (Oh yeah, this reminds me we need
> a delete button).
> 
> 2) No need to write code to limit the total number of queries allowed per user
> and write documentation or HTML to inform people about the limit (we don't want
> to support unlimited watched queries because of resources).

True, on the other hand we can sort of trust people on this I guess.
But still I am okay with this.  It just makes it more important to add
an OR syntax.

>>> +
>>> +    <div class="col-lg-10">
>>> +        {% for qn, msgs in queues.items %}
>>> +            <h3>Queue: {{ qn }} [{{ p }}]</h3>
>>> +            <ul class="panel" id="patches">
>>> +            {% for patch in msgs %}
>>> +                <li><a href="/{{ p }}/{{ patch.get_series_head.message_id }}/">
>>> +                  <span class="fa fa-lg {% if patch.has_replies %}fa-comment-o{% else %}fa-ellipsis-v{% endif %}"></span>
>>> +                  {{ patch.subject }}
>>> +                  </a></li>
>>> +            {% endfor %}
>>> +            </ul>
>>> +        {% endfor %}
>>
>> Since this is actually per series, maybe we want to format it like the
>> series-list page?
> 
> Ultimately this feature is not per series, we want it more flexible.
> Maintainers do tweak things after applying a series: dropping individual
> patches that are not mergable, fixing typos in commit messages or doing small
> changes to the diff.  Sure they can do it locally, and we can be simplistic on
> patchew.org, but these are also requested by the downstream maintainer tasks in
> Red Hat which is currently possible with patchwork. So I want to make it
> supported on patchew as well. The idea is adding a "drop" and an "edit" button at
> each patch in /my-queues. "Drop" is obvious, for "edit", it may give the user a
> page to edit the patch and a patch-to-patch is generated and saved as
> Queue.maintainer_tweak.

I think this is independent from making the queue display per series,
however.  That is, I would rename the Queue model to QueuedSeries, and
then we could add a QueuedPatch model and a new /my-queues/MESSAGE-ID/
detail view to do these "tweaks".

Paolo