[PATCH 00/11] Add labels support

[Don Zickus]

On Thu, Sep 13, 2018 at 05:12:30PM -0600, Stephen Finucane wrote:
> 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?

We don't use the counts, we prefer the values instead.

> 
> > > 
> > > > *  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.

Well, our bugzilla uses approval flags to coordinate inclusion of patches.
Those are combined with patch acks to pass the 'patch ready' rules.
> 
> > 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.

Yes, and our maintainers do.

> 
> 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

Yes.

>    2. User B sees patch, replies with a review and uses a CLI tool to set
>       a 'needinfo' flag against the patch (comment?)

No, user B only replies.

The maintainer processes and tags the patch and then processes and tags the
replies.  More replies, more tagging.  Rinse and repeat.  Stupid and simple.


>    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.

I might be confusing you with our complicated workflow, but our use of
labels/tags is super simple.

Only a maintainer with ACLs can set an arbitrary key:value tags.  It is that
simple.  Our various keys get a little complicated, but we standardized over
the years.

I think the complexity is coming from trying to map our generic key:value
tags to the definition of 'tags' you gave plus 'checks' plus some new
definition of 'labels'.

Honestly, throw out your idea on how to implement labels in the other thread
and I am willing to bet that covers our use case. :-)

Cheers,
Don