Wayland and Weston in Patchwork

[Pekka Paalanen]

On Tue, 4 Nov 2014 12:22:16 -0800
Bryce Harrington <bryce at osg.samsung.com> wrote:

> On Tue, Nov 04, 2014 at 01:14:20PM +0200, Pekka Paalanen wrote:
> > Hi all,
> > 
> > some of you already know that we now have
> > http://patchwork.freedesktop.org/project/wayland/list/
> > to keep track of the patches sent to wayland-devel mailing list.
> > 
> > Thanks to Daniel for setting that up. :-)
> 
> Ditto thanks Daniel, and thanks pq for this guidance in using it!
> 
> > You change the patch status by clicking the patch link in the list, and
> > on the following page you should have box titled "Patch Properties"
> > near the top. If you don't see that, you don't have permissions to
> > change it (e.g. not logged in, or unregistered email address).
> 
> There are several defined states:
> 
>  New
>  Under Review
>  Accepted
>  Rejected
>  RFC
>  Not Applicable
>  Changes Requested
>  Awaiting Upstream
>  Superseded
>  Deferred
> 
> Some of these seem obvious, others less so.  I think it would help to
> have a glossary of what they mean, so we use them consistently.  I poked
> around but didn't see one.

I don't really know those either.

> In particular, if I review someone else's patch and feel it is fine to
> be landed, how do I mark it to signal to the Committers "this is ready
> to land"?  Should I mark it Accepted?  Do I Delegate it to one of the
> Committers?

That's part of the process we still need to figure out.

> When is Under Review used?  Looking at the patchwork source this state
> and New are the two 'action_required' states, so perhaps Under Review
> should be for incoming patches that need someone to review them?

Ah, you looked for the definition of "action_required", thanks. So
"Awaiting Upstream" really does not belong to the action_required
category?

That makes "under review" sound like a maintainer has decided to review
the patch and has reserved it for himself, but has not reviewed it yet.
Not sure we need the distinction to "New".

For now, let's not play too much with the states. I want to keep all
patches that need maintainer actions (review or even just commit) in
the default list, i.e. action_required. We should probably just leave
everything as "New" until they are not pending on
upstream/maintainer/review anymore.

I have taken "Accepted" to mean that the patch is committed upstream,
i.e. pushed. Not the least because we have a git hook assigning
"Accepted" automatically on push. So, set state to "Accepted" only when
you push it.

"Superseded" and "Rejected" should be obvious.

The others I'm not sure what we would use them for.

I suppose "RFC" could be for "just sending this idea, in case anyone
has any comments, but no need for maintainers to react at all unless
they are interested".

All in all, we should probably be fine using just
New/Superseded/Accepted/Rejected.

> > Shortcomings:
> > 
> > There are several features we would like to see in Patchwork but AFAIK
> > are not there (yet?). Patchwork does not recognize re-submissions so
> > that it could automatically set a patch as "Superseded".
> 
> Might be nice if it permitted project-specific states, or even just a
> richer set of states to track things like if the patch has passed the
> testsuite, and to differentiate between okay-in-concept reviews and code
> quality reviews.

I'm not sure we want to track such details in Patchwork. After all, all
the details and discussion happens in the mailing list. We need to keep
the Patchwork maintenance burden as low as possible.

> > It does not maintain patch sets. You can create "Bundles", but so far
> > those are just named collections of individual patches, and you need
> > to create them manually.
> 
> If I create a Bundle of patches (e.g. Derek's large transform patchset),
> does that help others?  Or are Bundles just personal?

It does not directly help others, it does not automatically group things
in the list or anything. It just gives you a link you can share, that
includes just the patches in the bundle. Something like that.


Thanks,
pq