[U-Boot] Notes from the U-Boot BOF Meeting in Geneva 2012/07/12

[Graeme Russ]

Hi Detlev,

On Tue, Jul 17, 2012 at 7:30 AM, Detlev Zundel <dzu at denx.de> wrote:

>
> * Conflict resolution: setting up a moderator procedure for
>   unhappy submitters
>
> A recent occurence on the mailing list where contributers were sent
> through multiple rounds of patch submissions for non-obvious reasons was
> used as a ground for discussing non-technical aspects of patch
> submissions.  It was especially discussed that the privilege of being a
> custodian in U-Boot also brings obligations that should be defined more
> clearly.  Essentially we would like to see somewhat "enforceable" rights
> for the "motor" of the project, i.e. for patch submitters.
>
> For this we should extend our documentation to clearly cover
> responsibility of custodiands.  Require making constructive (testable)
> suggestions on how to get patch accepted when nak-ing patches for
> example.  This can also be enforced by patch submitters by waiting for
> clear and precise suggestions for re-work.
>
> It was also found to be a good idea to document people willing to act as
> "moderators" for unhappy contributers, i.e. who can look into problems
> and try to moderate.  (I would volunteer to be such a moderator).

For some first time submitters, it can be a very daunting (and discouraging)
process to get patches accepted - Especially if they are submitting a large
and complex patch-set to introduce a new board or arch.

I have in the past acted as a mentor for first-time submitters. This
involved taking the submission offline for a couple of revisions. This
gives the submitter some room for error and an opportunity to have the
submission process explained in direct relation to their patches without
the often blunt and confusing feedback that arises on the list. Of course
this has it's own set of problems as it can result in strange revision
numbers when the patches come back to the list.

I'm more than happy to continue doing this on the understanding that it
can lead to some 'jitter' in the submission process.

> Currently the lack of any reaction whatsoever was identified to be a
> very discouraging sign for contributors. One thing we could do is to
> declare a "soft" time-limit (two weeks) that patches need to be looked
> at.  After this time-limit, one could declare "backup-custodians" to
> push patches to or merge patches into some "-staging" branch.  (What to
> do with that branch then?) For this to work, custodians would need to
> announce "off-times" which currently is not general consensus.

I like this idea in principle. In order to work, I think there needs to be
two or three 'top-tier' custodians who will arbitrarily assign 'stale'
patches to another custodian. And maybe one top-top-tier custodian (Hi
Wolfgang ;)) in case the patch goes very stale.

I think a couple of ideas could be employed:
 - If a custodian is backlogged (or otherwise unable to immediately review
   the patch) they should issue a quick reply with an estimated date that
   they will be able to review the patch. At least then the submitter is
   not left in the dark
 - If a custodian simply does not have time to review a patch, they should
   request for another custodian to take ownership. If no other custodian
   responds, the top-tier custodians can take action.
 - If a patch is submitted outside the merge window, or the patch requires
   major rework such that the custodian does not think the patch will be
   included in the current merge, a clear message needs to be provided to
   the submitter (The recent ZFS patches are a good example)
 - If the patch is a 'miscellaneous' patch (i.e. no clear custodian) then
   whichever custodian 'claims' it should send a message back to the list
   stating so - That custodian is then responsible for communicating with
   the submitter regarding the status of the patch (via the list or offline
   if required - see my mentoring comments above)

Whatever we do, each patch needs to have clear indications of:
 - Which custodian is responsible for it
 - When it will get reviewed
 - What the review state is (approved, awaiting changes, rejected etc)
 - When it will get merged (current release or next release)

> * Patchwork pros / cons
>
> While talking about the patch submission process it became clear that
> the handling through patchwork still has some unsolved problems, the
> most obvious being the large backlog of patches in the archieve.

I haven't had a chance to do my usual patchwork housekeeping for a while.
When annoying thing about patchwork is that old revisions do not get
automatically marked as superseded.

> It was agreed that automatic marking of changes as accepted when changes
> hit mainline would be a good thing.  There is some info on our wiki
> available, although it is not clear who uses them:
>
> http://www.denx.de/wiki/view/U-Boot/Patches#pwclient and
> http://www.mail-archive.com/patchwork@lists.ozlabs.org/msg00057.html
>
> A tool to automatically mark patches as superceded on newer versions
> was also suggested to be very valuable.

As I said :)

Also, if one (and only one) maintainer is Cc'd on a patch, it would be
nice is it was automatically assigned to them. Same goes for tags in the
patch subject - there should be a way to automatically assign a fair
number of patches.

> Unanswered questions:
>
> - What can patman do automatically?
> - Can gerrit help the workflow?
>
> During the discussion it showed that many people believe that patches
> outside the "regular" merge window are handled in-efficiently ending up
> as large piles in patchwork.  It was agreed that rather then being a
> "accept / don't accept" division, it should be a "accept into mainline /
> accept into -next branches" split hopefully keeping the flow of incoming
> patches more uninterrupted.  For this to work, every custodian would
> open his own "-next" branch and start merging patches from the mailing
> list resulting in patchwork becoming cleaner during "bug fix" phases.

And this is where patchwork fails us - The list of states is extremely
limiting

> It was discussed whether to do some "automatic" merging of these
> per-custodian trees into a central next, but majority of people believed
> that the patch handling process should remain as unchanged as possible
> in sync with the "principle of least surprise".

I agree that automatic merging is a 'Bad Thing(tm)'. But one thing I notice
(and I don't know if this is a recent thing) but there seems to be a case
of zero merge activity up to the closing of the merge window and then a
rash of merging just prior to the RCs. I favour a more continuous merge
strategy.

> * Continuous integration
>
> Discussing such automatic merges, the need for continous integration
> became apparant.  As mid- to longterm goals we would like to see
> automatic builds shifting the requirement to "do MAKEALL" from
> individual posters to (cheap) machine time.  One obstacle on this way is
> the complexity of automatic builds for different architectures,
> i.e. which toolchain to use and how to use them correctly.

'Bring out your nightly builds!'

I very much like the idea of continuous integration - The number of time
that a build is breaking only to be picked up weeks after the offending
commit is getting a bit annoying.

> It was agreed to be a good thing to collect Mike's cross-toolchains on
> denx.de and ease running MAKEALL.  Especially add more switches to
> MAKEALL for toolchain selection, log-, build- directories.  Turn
> interpretation of environment variables into switches to be more in line
> with "good unix practice".
>
> Simon Glass mentioned that he is working on "buildman" which he will
> present on the mailing list in due time.

Looking forward to it

Regards,

Graeme