Machine-scope maintainers for OpenBMC

[Andrew Jeffery]

On Tue, 2017-10-17 at 11:25 -0700, brendanhiggins at google.com wrote:
> > Hello!
> > 
> > I'm wondering whether we should jot down the maintainers of machines
> > supported by OpenBMC, and if so, where? With respect to the latter, one
> > candidate might be the MAINTAINERS file in openbmc/docs.
> 
> I agree with that idea; this is what the MAINTAINERS file was supposed to be
> for. The reason I chose the syntax @repo-name://*, was that the '*' at the end
> is supposed to be a file matcher path (this is mentioned in the header of the
> document); however, at the time there were no sub-repo owners, hence all the
> entries were @repo-name://*.

Indeed! Given it's a per-repo description it may get a bit unwieldy but we'll
see how it turns out. Primarily, my concern is that we will have to at least
describe paths in the kernel repo, paths in the openbmc/openbmc repo, and
possibly a bunch of others. I'm curious if there isn't a more effective way to
resolve this, but there's probably not as that's how the code is structured
(and if we want to have something like get_maintainers.pl, then it needs to
know about these paths).

> 
> Nancy and I talked to Brad yesterday and he mentioned that he would like to have
> a way to enforce sub-repo permissions, since Gerrit only enforces permissions on
> a repo level. We, Google, have recently started doing automated reviews to
> verify that Google internal changes had owners that we could go back to durring
> a merge/rebase.

Can you expand on what you are talking about here? You've piqued my interest.

> It should be pretty straightforward to do something similar for
> enforcing sub-repo reviewers/maintainers: we could have a bot that would listen
> for all new changes (similar to the Jenkins bot) and then add the appropriate
> reviewers based on the MAINTAINER file entries; it could also then vote +1/+2
> once one or more of the appropriate reviewers have signed off.

Right. There are gerrit plugins that can auto-assign reviewers based on
git-blame and such, maybe we could also massage one of those. Is there much
benefit in the bot assigning a score?

> 
> Gerrit documentation for doing this type of thing can be found here:
> https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html
> 
> > 
> > A related question is: Who are the machine maintainers? Off the top of
> > my head I feel the following machines machines match with people:
> > 
> >     meta-evb/meta-evb-raspberrypi                  Yi TZ Li <shliyi at cn.ibm.com>
> >     meta-evb/meta-evb-aspeed/meta-evb-ast2500      Joel Stanley <joel at jms.id.au>
> >     meta-x86/meta-mellanox/meta-msn                Mykola Kostenok <c_mykolak at mellanox.com>
> >     meta-x86/meta-quanta/meta-q71l                 Patrick Venture <venture at google.com>    , Rick Altherr <raltherr at google.com>
> >     meta-openpower/meta-ingrasys/meta-zaius        Xo Wang <xow at google.com>
> >     meta-openpower/meta-ibm/meta-firestone         Lei YU <mine260309 at gmail.com>
> >     meta-openpower/meta-ibm/meta-romulus           Lei YU <mine260309 at gmail.com>
> >     meta-openpower/meta-ibm/meta-witherspoon       Brad Bishop <bradleyb at fuzziesquirrel.com>, Andrew Geissler <geissonator at gmail.com>
> 
> I proposed a change that adds the above entries in the syntax that I suggested
> above: https://gerrit.openbmc-project.xyz/#/c/7409/

Thanks, I'll take a look.

> 
> > 
> > Less obvious are the following machines:
> > 
> >     meta-openpower/meta-ibm/meta-palmetto
> >     meta-openpower/meta-ibm/meta-garrison
> >     meta-openpower/meta-rackspace/meta-barreleye
> > 
> > Yet another related question is what do we want to do with machines
> > that don't have an obvious maintainer?
> > 
> > In the case of Palmetto I imagine we could find someone willing to step
> > up (/me ducks). Garrison I'm unsure about, and for Barreleye I've had
> > kernel patches on the list for several months with not a lot of
> > interest in them, which indicates to me that we could probably drop it.
> > 
> > As some background, Joel and I were recently discussing machine
> > maintainers with respect to the dev-4.13 kernel effort. The approach
> > Joel kicked off for dev-4.13 was to start from scratch against upstream
> > and distribute the forward-porting amongst owners of patches in dev-
> > 4.10. That is, if you have patches in dev-4.10 and that are not in
> > upstream v4.13, you should rework and send them for us to apply to dev-
> > 4.13.
> 
> So this is something we have been trying to deal with internally as well, I came
> up with a merge/rebase document that described a system for keeping track of
> internal changes. At its core, it relies on a set of commit footers:
> 
> * Effort - used to specify who the work is for. This makes the project
>   responsible for supplying people to address problems supporting these commits.
> * Origin - Specifies the original version of an internal commit; used when an
>   internal commit is rebased or cherry-picked.
> * Dropped - Specifies that a commit is to be dropped in any rebase.
> * Upstream - Specifies that a commit is to be dropped if the commit can be found
>   in the branch being rebased to.
> * Git-repo - used in conjunction with Upstream, used to specify a non-mainline
>   upstream repository, this should probably always be an upstream maintainer's
>   for-next repository.
> * In-review - used to specify where the most recent version of a patch is being 
>   reviewed upstream.
> * Maintainer - used in exceptionally rare circumstances to identify who is
>   responsible for maintaining an internal patch.
> 
> We also have an internal GOOGLE_MAINTAINERS file for files that are not
> upstream, rather than using the Maintainer footer for everything.
> 
> The idea is, that whenever we do a rebase, we can run a script that looks at the
> above footers and should be able to resolve most of the issues; either by
> automatically dropping the commit, when it has been superceded, or contacting
> the appropriate owner when the issue needs to be resolved manually.
> 
> Let me know if this sounds useful, and I will get the document in a format that
> I can share via email.

I'd certainly like to explore this more, so if you can share please do! I'd
like to get feedback from people on the list (primarily those affected,
particularly Joel, and I expect Brad will be interested).

Thanks for the detailed reply, clearly it's something that's bothered people
besides Joel and I. If it's bothered people beyond us and Google, I'd love to
hear your solutions if you've got so far as having any.

Andrew
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20171018/5a5ae3bb/attachment.sig>