[PATCH 00/10] Patchwork Autodelegate patches

[Laurent Pinchart]

Hi Stephen,

On Thursday 24 December 2015 12:45:47 Finucane, Stephen wrote:
> On 28 Nov 10:14, Mauro Carvalho Chehab wrote:
> > This patch series contain the non-LinuxTV specific patches,
> > mainly the auto-delegate patches written by Laurent Pinchart.
> > 
> > Those got ported to patchwork upstream.
> > 
> > Please notice that I'm still fighting to finish the migration
> > from Django 1.4 to Django 1.7. So, this patch series is not
> > fully tested.
> > 
> > The auto-delegation patches are authored by Laurent, and were
> > rebased by me to apply under the upstream branch. They're
> > working for a long time at LinuxTV.org, but the rebase might
> > cause some troubles on them.
> > 
> > So, please test.
> 
> [snip]
> 
> OK, so I've tested this. The rebase wasn't as bad as we feared and
> while there are a few issues they're nothing serious (I'll reply to
> these separately). However, I do have a design question I'd like to
> pose to you, Laurent and anyone else interested, before I start
> merging this.
> 
> My question is this: is there any advantage to providing a larger
> "hook" system in patchwork? I ask this because this is the second
> series that attempts a hook-style feature (the other being Damien's
> git-send-email handler [1]). Both of these features do special actions
> based on a patch's content: delegating a patch in this case and
> ignoring it in the case of the git-send-email handler. I wonder if
> there would be any advantage to making this more generic. For example:
> 
>     if patch.subject contains "models: " then set delegate to "Bob"
>        |___________| |-----------------|      |--------------------|
>           element      match criteria                  action
> 
> Some other examples:
> 
>     if patch.content contains "+++ hello_world.py" then set state to
>         "rejected"
>     if patch.author equals "example at example.org" then ...
> 
> This would provide an immense amount of flexibility, assuming this
> flexibility would be beneficial. It would also allow us to avoid
> creating Django migrations for each new feature of this sort, which
> will keep the code simpler and reduce the load on the sysadmins
> maintaining patchwork instances (we'd only need migrations if we
> didn't scope out the actions sufficiently initially and ended up
> needing new actions.
> 
> However, all this is me thinking out loud for now, and I'm busy enough
> reviewing patches and trying to get something approaching series
> support into upstream (plus, you know, my day job) to volunteer
> implementing this. As such, if you agree with my idea but don't think
> it worth the effort (or just flat out disagree) then I'm happy to
> give my review comments on this with an eye to merging it shortly. If
> you do agree though, maybe we should hold off merging this until we
> explore these options?
> 
> Thoughts?

I generally like implementing features in a generic way, but as an occasional 
patchwork user I have a hard time judging how useful it would be. The trust 
issue that Johannes raised should also be considered here, to make sure that a 
malicious user wont be able to take advantage of either the criteria or the 
action to attack the patchwork host.

-- 
Regards,

Laurent Pinchart