[Patchew-devel] [PATCH v2 00/12] Queue, MAINTAINERS and watched query
Fam Zheng
fam at euphon.net
Wed Nov 28 14:34:53 UTC 2018
Paolo: I switched the email provider for my personal email address, migrated
data before you replied to v1, but the MX records were bogus until later, so my
new INBOX lost your replies. But I'm replying here:
> > 4) An event is generated upon the addition, which is configured as a source for
> > email notifications just like testing reports. In the template, To/Cc addr
> > of the original message is tested. Depending on if or not the user's email
> > is in it, an email from no-reply patchew org to the user's address is sent,
> > in reply to the original message, to catch attention from the users INBOX.
>
> Can you explain how this part would be configured? Especially the
> "depending if or not the user's email is in it" part.
It's possible to be done using the template markups I believe. In an email
config for MessageQueued event, something like this:
```
{% if queue.name == "watched" and queue.user.email not in message.recipients %}
Hi, this message ({{ message.subject }} by {{ message.sender }})just landed in
your watched queue on Patchew but your address is not in the recepient list.
{% else %}
{{ cancel }}
{% endif %}
```
> > 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.
>
> 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?
>
> > Do we want the applier to create and maintain a git tree/tag for each queue?
>
> Maybe later, but it's not required.
>
> > What kind of user facing commands do we want on patchew-cli? Downloading and
> > applying a queue?
>
> As a start, even just listing the git tags should be enough for the user
> to script it. Maybe a generic "formatted search" like "git log
> --pretty=format:..."?
More on this in reply to your /my-queues page comment below.
> > 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).
So I've changed the model field to ForeignKey to make it one-to-many, but the
code still only maintains at most one record.
> > +
> > + <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.
v2:
- Rebase to current next
- Put emit_event in the right patch. [Paolo]
- Use OneToMany relation between User and WatchedQuery. [Paolo]
- Squash the query_test_message patch. [Paolo]
I refreshed my queue branch from a year ago, in a slightly different way. This
more or less makes it possible to realize what Markus asked for: watch for
patches touching certain files (as indicated by MAINTAINERS file) and
automatically notify him if not already in Cc.
The user story basically consists of four parts.
1) A logged in user creates a "watched query", by accessing (once we merge this)
http://next.patchew.org/search?q=maint%3Ame+%21cc%3Ame and clicking the "Watch
query" button. The search query will be saved for the user in the database.
2) Once a new patchset is uploaded by the importer, and post-processed by the
applier, which also collects the associated maintainers by invoking
.scripts/get_maintainer.pl.
3) The watched query is run against this new patchset. If it is included in the
query, it is automatically added to the user's queue named 'watched'.
4) An event is generated upon the addition, which is configured as a source for
email notifications just like testing reports. In the template, To/Cc addr
of the original message is tested. Depending on if or not the user's email
is in it, an email from no-reply at patchew.org to the user's address is sent,
in reply to the original message, to catch attention from the users INBOX.
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. Any comments on how we should do
that? Of course the role separation and fine-grained permission is only a small
part of the problem, the bigger one is how to provide better interface and hide
the email config and templating details.
Maybe the solution is to package the feature (into that "watch query" button)
and just send the notifications when it makes sense?
Do we want the applier to create and maintain a git tree/tag for each queue?
What kind of user facing commands do we want on patchew-cli? Downloading and
applying a queue?
Other comments, ideas, feedback?
Fam Zheng (12):
scripts: User download.patchew.org for example DB
mod: Pass request in render_page_hook
Generalize Review model to Queue
maintainer: Queue operations in extra ops
model: Introduce WatchedQuery
Add "Watch query" button to search result page
maintainer: Update watched queue when getting new message
www: Add /my-queues page
maintainer: Add MessageQueued and MessageDropping events
model: Add and populate Message.maintainers
search: Introduce maintained-by:NAME term
maintainer: Add maintainers status to series details
api/migrations/0042_review_to_queue.py | 19 ++
api/migrations/0043_auto_20181120_0636.py | 40 ++++
api/migrations/0044_watchedquery.py | 26 +++
api/migrations/0045_message_maintainers.py | 22 ++
api/models.py | 20 +-
api/search.py | 31 ++-
mods/footer.py | 2 +-
mods/git.py | 4 +-
mods/maintainer.py | 233 +++++++++++++++++----
patchew-cli | 31 ++-
scripts/developer-setup | 2 +-
www/templates/base.html | 3 +
www/templates/my-queues.html | 82 ++++++++
www/templates/series-detail.html | 30 ++-
www/templates/series-list.html | 8 +
www/views.py | 2 +-
16 files changed, 493 insertions(+), 62 deletions(-)
create mode 100644 api/migrations/0042_review_to_queue.py
create mode 100644 api/migrations/0043_auto_20181120_0636.py
create mode 100644 api/migrations/0044_watchedquery.py
create mode 100644 api/migrations/0045_message_maintainers.py
create mode 100644 www/templates/my-queues.html
--
2.17.2
More information about the Patchew-devel
mailing list