[PATCH 00/11] Add labels support

[Stephen Finucane]

On Thu, 2018-09-13 at 16:48 -0400, Don Zickus wrote:
> On Wed, Sep 12, 2018 at 04:37:21PM -0600, Stephen Finucane wrote:
> > > > I see labels as different to tags. Tags are key:value pairs, generally
> > > > extracted from a message body, while Labels are simply values initially
> > > > stripped from the subject. We could build a unified model that supports
> > > > both, perhaps by making the value aspect of Tags nullable, but I'm not
> > > > sure if that's something we'd want nor how this would be handled in the
> > > > UI/APIs. What are your thoughts here?
> > > 
> > > Hi Stephen,
> > > 
> > > Our internal workflow is a mixture.  We called everything 'tags' for
> > > consistency but really our data is either:
> > > 
> > > *  key:value pairs extracted from message Body (bugzilla, acked-by, backported
> > >    commit id)
> > 
> > Yeah, this what I envision tags being for. These are mostly human
> > written or at least human readable and would be something you'd want to
> > count.
> 
> Well counting is good for acks, but when storing a Bugzilla number or a
> commit hash, it doesn't work so well. :-)

Good point. We probably don't want to count every type of patch. Maybe
that should be a configurable?

As an aside, are tag counts any way valuable in the list UI? I wonder
would it be better showing a simple label (a la GitHub issue labels) if
at least one has been shown?

> > 
> > > *  arbitrary key:value pairs from automation tools (build id, test job id,
> > >    status of comment), nothing attributed to an email message body or easily
> > >    re-created by re-reading emails.
> > 
> > This sounds like checks [1]. I know that these wouldn't exactly map to
> > what you have currently, but would these be suitable solution? If not,
> > what gaps do you see that would need to be closed?
> 
> The build id and test job id can mapped to a url and a check state.
> 
> If we want to cache bugzilla info to avoid the constant remote database hit,
> that might be a challenge.  Though maybe you could map it to a 'check'
> state.

What kind of stuff are you caching? The state of the BZ itself? BZ has
a _lot_ of possible configurations and everyone uses it differently.

> However, a big part of our process is handling patch replies.  Yes acks/nacks
> are handled by tags above.  But when someone asks a question, we usually tag
> the 'reply' with Neddinfo.  The idea is we block the acceptance of the patch
> on the patch poster replying to the question (within reason at the
> maintainers discretion).  Of course the followup is tagged with
> 'Needinfo-rescinded'.
> 
> The needinfo tags then become part of our 'patch ready' rules.
> 
> The end result is every email reply is tagged with _something_.  This also
> plays into our 'patch ready' rules.  If a maintainer is about to commit a
> patch and a last minute reply comes in, it gets blocked until processed.

I'm guessing you have tooling wired up to check this stuff? How does
this needinfo tag get set? Via something in an email or via a CLI tool
(presumably a pwclient variant)? If the latter, we could have an ACL
model for labels and allow users to set this stuff via pwclient/git-pw
locally.

I think I asked for this before, but a sample workflow for a simple
patch would be very helpful in understanding these requirements.
Something like:

   1. User A submits patch to mailing list
   2. User B sees patch, replies with a review and uses a CLI tool to set
      a 'needinfo' flag against the patch (comment?)
   3. ...

(Apologies if you sent this to me already - I couldn't spot anything in
my inbox).

> Overkill, sure.  But with the amount of work we have, it is easy to lose
> track of email replies (or purposely ignore them).
> 
> That would be the biggest gap I think.  Hence why we were aiming more for
> arbitrary labels, but I also see that as to generic and easy to get wrong.
>
> Does that make sense?

Yeah, I'm happy to support some of this stuff but I do want to make it
somewhat generic. The question is how generic we can get while still
fulfilling your requirements, heh.

Stephen

> Cheers,
> Don
> 
> > > Now I know you were not excited about arbitrary tags/labels when we first
> > > spoke, so we have been focusing on the first type of data.
> > > 
> > > But ideally we would like both (or actually the second type covers the first
> > > type :-) ).  And I think the second type of data covers what Michael E. was
> > > describing for his needs (he called them labels).
> > > 
> > > We are ok with ACLs for setting them (as long as our bots can get
> > > permission).
> > 
> > For what it's worth, there's an ACL of sorts in place for checks: you
> > need to have a Patchwork account and the username of the user that
> > creates the check is stored and displayed in both the UI and API. I'm
> > hoping to extend this to add permissions so only certain anointed users
> > (read: bots) can create checks but I just haven't gotten around to that
> > yet.
> > 
> > > Does that make sense?
> > > 
> > > Cheers,
> > > Don
> > 
> > Cheers,
> > Stephen
> > 
> > [1] https://patchwork.readthedocs.io/en/latest/usage/overview/#checks
> >