[Xen-devel] Survey Results - Part 1 : Improving Xen Project Governance and Conventions

[Lars Kurth <lars.kurth.xen@xxxxxxxxx>]

Dear Community members,


we recently did survey amongst maintainers on a number of issues that were 
raised during the Xen Project developer meeting. I had 12 responses and wanted 
to share the output + some initial analysis for further discussion. I got 
really rather a lot of feedback and I tried to condense as much as possible. In 
many cases, I reworded some quotes to ensure  privacy. In some cases I omitted 
some quotes/suggestions to make this more readable.

For readability, I am going to break the replies into the 4 parts, alongside 
which the survey was arranged.

1) Hierarchy of maintainers in the xen.git MAINTAINERs file
2) Trust amongst different stakeholders in the peer review process
3) Other related Governance Issues
4) Possible Solutions

Results and comments are marked with | under each question. Followed by an 
analysis, which is marked with !

Also, section 4) was based assumptions and may be incomplete. I will try and 
summarise the replies, but there is little common ground it appears. A summary 
of the later parts will probably be sent out towards the end of this week.

Best Regards
Lars

= Survey Questions : Improving Xen Project Governance and Conventions =

== 1) Hierarchy of maintainers in the xen.git MAINTAINERs file ==

Our governance document (see http://www.xenproject.org/governance.html - 
under Roles/Maintainers) assumes that there is a fairly clean mapping 
between components and maintainers. 

However, maintainership for xen.git appears to be more complex in reality:

* We have situations where a maintainer maintains just a sub-portion 
  (maybe even a single source file or more fine grained) of a larger 
  component. For example ''tools/a/b.c'' is maintained by M-bc, while 
  ''tools/a'' is maintained by M-a1 and M-a2, while ''tools'' is 
  maintained by M-tools.

* In this case, we have a de-facto hierarchy of maintainers, with some 
  maintainers having responsibility for an entire component (in the example 
  above M-tools is responsible for a superset of code, that M-a1 and M-a2 
  are also responsible for, who in turn are responsible for a superset of 
  code which M-bc is responsible for) . 

Q1.1: Do you agree/disagree with the statement that we do have ânested 
maintainer ownershipâ as described above in the project?

| 11 of 12 agree with this statement
| 1 disagrees and states that "There is no nesting or hierarchy. Changes 
| to tools/a/b.c can be approved by any of M-bc, M-a1, M-a2, or M-tools"

! There seems to be broad consensus, that today we do have a hierarchy

* One common practice in many open source projects in such situations is 
  that there is a hierarchy of trust (typically a social construct). This 
  exhibits itself in the following sense...

** An ACK from M-tools is normally required, which sometimes means that 
   M-tools will re-review some of the code already reviewed by maintainers 
   of sub-components (i.e. M-a1/M-a2 and M-bc).

** In some cases M-tools may trust M-a1/M-a2 enough and delegate part of 
   his/her responsibility (in other words M-tools bestows ACKs by default, 
   or there is an explicit statement to committers to accept a review by 
   M-a1/M-a2 as an ACK).

** The same would be true for M-a1/M-a2 and M-bc respectively.

Q1.2: Do you agree/disagree with the principle of âdelegated trustâ as outlined 
example above?

| 10 of 12 agree with this statement
| 1 asks a clarifying question and states "I think it's OK for M-tools 
| to give an ACK, without reading a patch, simply because M-a1 has reviewed it"
| 1 disagrees, as it conflicts with the corresponding answer to Q1.1  

! There seems to be broad consensus. 

Q1.3: Do you agree/disagree with the statement, that such situations are not 
well 
described in our governance?

| 1 of 12 has no opinion (has not read the governance)
| 10 of 12 agree with this statement
| It is not mentioned, no.  I'm not sure it needs to be mentioned in
| governance; A guide for maintainers might be a better place for it.
| 1 disagrees, as it conflicts with the corresponding answer to Q1.1

! There seems to be broad consensus and maybe a guide for maintainers
! may be the best way forward.

Q1.4: If you are a maintainer which owns a subset of a component owned by 
another 
maintainer, do you feel disempowered by this set-up. If so, explain why.

* Situations where we have several maintainers for the same component, one
  of which owns a subset of another maintainer, can create complexities in
  the review process. Consider the following scenario:

  S is a contributor, M1 is a maintainer for the entire component, M2 
  maintains a subset of M1âs component. 

  * S sends a patch (which mainly affects M2âs subset, but contains 
    some small portions owned by M1)
  * Three days later M2 acks it
  * A few weeks pass
  * M1 provides additional feedback
  * S updates the patch, and the pattern is repeated
  * After a month is still uncommitted, even though M2 performed a prompt 
    review several times

| 7 of 12 has no opinion (as they are not in this situation)
| 5 disagree - here are some more detailed comments.

| I do not feel disempowered.  I have always seen my ACK on code that 
| I maintain as "To be committed, unless a superset maintainer disagrees".
| If anything, I felt _encouraged_ by having superset maintainers also
| double-check patches.

| Jan is quite deferential to me in the code I maintain; he
| will often say, "Well, I don't like X, but it's really up to you as
| maintainer", which is empowering.

| Another point that was raised was that "some maintainers responsible for
| a part of a component are not very active, and sometimes the only way to 
| get a change approved is to rely on the main maintainer (this may be indeed 
| a consequence of the "hierarchy" mentioned in Q1.1)

! Nobody seems to feel disempowered by a hierarchy

Q1.5: Have you come across such a situation, and if so, how do you believe it 
should be handled? 

| 2 of 12 have not come across such a situation
| 9 of 12 have come across such a situation
| 1 of 12 disagrees with the premise, as everyone has the same "influence over 
| code". Following up, there was a recognition though that we are a meritocracy
| and that some maintainers input has more weight of others due to more
| experience and work done.

| Some relevant quotes:

| I haven't seen examples of the "nested maintainership".  More commonly,
| there are aspects of code that one maintainer may understand better than
| another. 

| I think you are asking two questions here.  The first is about
| secondary review: I have certainly seen cases where M2 has acked but
| M1 required further changes.  I have been both M1 and M2 in that
| scenario.  Usually M2 has not objected to M1's request for further
| changes.  OTOH, if M2 disagrees with M1, they should say so.
|
| The second is about the delay in M1's review.  In that case:
| - Either S or M2 should ping the acked patch after a week or so.
| - If M1 has no time to review within a few weeks then they _must_
|  delegate.  They can either do a lighter-weight review, checking
|  the overall structure but skipping the fine detail, or take M2's ack
|  as enough by itself.  How much to delegate will depend on
|  context.

| If we remove "A few weeks pass" then I am absolutely certain I have.
| If we include "A few weeks pass" then I am reasonably confident this 
| must have happened. ... In an ideal world M1's review would be more 
| timely than "a few weeks" in any of these cases.

| With the 'maintainer hierarchy' being discussed above in mind, e.g., if
| M2 maintains tools/* and M1 maintains tools/a/* and the patches touches
| (however small) bits in tools/* we must wait for an ack from M2 (or from
| another maintainer of the same area). If that does not happens timely,
| then that's the issue, and fixing should be attempted at that level
| (i.e., the timing, not the hierarchy).

| With our current set of roles, M2 should escalate this problem to the
| committers, who should decide (on the basis of technical and other
| considerations) between (i) committing the code, or  (ii) M1's review
| is important and the patch does indeed need to be blocked until M1's
| comments have been made.

| Maintainers should respond in a timely manner. Committers should also
| clear patches in a timely manner. Better communication is needed even it
| is just saying "I'm busy at the moment and will come back as soon as
| possible".

! It appears that there is consensus, that we have seen such issues.
| However, the main issues we are seeing may be better resolved
! through better communication and more timely reviews.  

Q1.6: Do you accept, that such a situation can be frustrating/confusing for a 
contributor? 

| 11 of 12 agree
| 1 disagrees and believes that the core issue is that "we re mixing the 
| different hats people wear: committers only commit, but are also maintainers
| reviewers. Since we try to commit things only to areas where we're also
| maintainer for (unless changes cross boundaries), you can't really
| separate the different roles.

| A patch which gets acked by a maintainer (who is not a committer) should 
| have the same chances of being committed than a patch acked by a committer.
| That does not mean, that a maintainer can't or shouldn't review and
| comments on patches already reviewed or acked by his fellow
| co-maintainers. All it means is that, if the two patches does not have
| the same chances and probability to get in, that means there is an
| implicit hierarchical relationship which is not reflected anywhere,
| which is what could be confusing.

| Yes, this can be hard to deal with from a contributor point of view,
| because it gives the impression that either M1 lacks technical
| competences or C1 doesn't want your change committed for some unknown
| reason, so he just delays it by providing additional feedback.

| Absolutely.  It is up to M1 and M2 to make it clear to S what is going
| on and what, if anything, S should do about it.

! Again there seems to be broad agreement.

* In the Xen Project governance, we have the concept of committership, 
  which is defined as a maintainer with write access to the xen.git tree. 
  The governance states that Committers act on the wishes of maintainers 
  (via the maintainerâs ACK). This creates the impression that the 
  Committer role is âsecretarialâ.

* However in practice, due to ânested maintainershipâ, ACKs by maintainers 
  do not always mean the same thing. Consequently, Committers feel that 
  their role is not âsecretarialâ and perform judgements on whether 
  portions of patches need to be re-reviewed and whose ACKs are needed 
  before a patch can be committed. In some sense, committers perform a 
  value judgement on how much to trust individual reviewers/maintainers. 
  This can lead to maintainers feeling disempowered, and consequently not 
  stepping up to what is expected.

* Situations where committers share the maintenance of a component with 
  other maintainers who are not committers can create mismatches in   
  expectations during the review process. For example, consider the 
  following scenario:

  S is a contributor, M1 and C1 co-maintain a component, M1 is a maintainer 
  while C1 is a committer

  * S sends a patch
  * Three days later M1 acks it
  * Some time passes
  * C1 provides additional feedback
  * S updates the patch, and the pattern is repeated
  * S feels that he doesnât know whose advice to follow and gets confused
  * M1 may feel that his review comments are not taken into account and 
    reviews less subsequently

Q1.7: If you are a committer, how do you see your role? Do you agree/disagree 
with the statement that the Committer role is âsecretarialâ? 

| 8 N/As (of which some stated that it should be Yes)
| 2 No
| 2 Yes

! We are quite clearly split in the middle here. I am wondering, whether
! what is really going on here is that we do in fact mix different hats and
! ideas and do not always separate them clearly. In particular, because our 
! committers tend to also be the most experienced maintainers.

Q1.8: If you are a maintainer, do you think the current set-up which states 
that committership is âsecretarialâ, when in practice it is likely not, is 
causing problems to you. If so, please elaborate. 

| 5 No
| 3 N/A
| 4 Yes

Some interesting quotes

| If every committer insisted on doing a full review before
| committing, then there would be little point to having separate maintainers 
|  -- the committers would be the bottleneck rather than the maintainers.
|
| Perhaps the problem is committers not clearly separating their two
| roles.  They should put their maintainer hat on, and say, "If I were not
| a committer, would I review this?"  And then either review it or not.
| Then put their committer hat on and say, "If I were not maintainer of
| this, would I commit it?" And then either commit it or not.

| The problems you are describing seem to be to to with (a) nested 
| maintainership, (b) maintainers disagreeing, or (c) slow review, and not the 
| committer's role.

| I feel disempowered as I always know who has the last word, despite having 
| worked on the project for some years.

| This causes discomfort even to me who has been working on the project for
| almost 3 years. In practice it is not causing too much trouble for me 
| personally, as the committers I work with never use their committer status
| to trump my decision -- they are willing to commit patches they don't
| agree with. But, I do understand that other contributors have had issues
| with this. I would still like the governance to match reality no matter what 
| model we end up with.

| Yes, currently maintainers have no right, so why do we still need to review.
| There should be right and duty for maintainers.

! We do appear to have some issues in this area, which will need to be explored
! further. In particular because the earlier questions Q1.1 - Q1.4 imply
! that some sort of hierarchy based on seniority is not seen as a problem by
! a clear majority of respondents. I am not quite sure how to go about this, 
! as this is clearly an emotionally charged issue. Setting expectations better, 
! may help, but I doubt it will fix the root cause. This, taken together with 
! answers to Q1.7 does also worry me: it implies a degree of inconsistency
! in how we work, that could be the root cause of any dissonance that was
! highlighted by the survey.

Q1.9: Do you think, that a more honest approach, that better reflects ânested 
ownershipâ of code in Xen and a âmaintainer hierarchyâ would improve trust 
between different stakeholders in a code review? If so, how? If not, explain 
why?

| 3 No
| 7 Yes (although some are not quite clear)
| 2 N/A (one was an objection to the term "more honest" - the other was "Both 
| Questions [1.8 & 1.9]only seem to make sense when committer,
| maintainer, and reviewer are all different people."

! On the N/A, Q1.9 the "more honest" should be deleted. The question still
! makes sense without and there was no implication that anyone was dishonest
! (it's a Germanism).

Quotes from people 

| No. If higher-level maintainers are not acting in trust to lower-level
| maintainers, this would be either because they should not be trusted, or
| because some higher-level maintainers perhaps have trouble letting some 
| things go and delegate. Neither will be changed by a clearer description 
| of the hierarchy.

| No. I don't think there is a lack of trust. There will always be
| situations where different maintainers see different issues or where
| they would like things to be changed in different ways. These
| differences must be discussed in an open way as they - as I think -
| are being discussed now. If a contributor is confused by this
| discussion he should say so and ask the maintainers for a final
| advice how to proceed.

| Yes, I think that "nested ownership" needs to be better represented 
| in the governance. We could try to setup the code structure in a way 
| that would make maintainer hierarchies easier to handle going forward.
|
| On the other hand a difficult relationship between a maintainer and a
| committer co-maintaining the same component should be avoided.

| Yes. I think it would help, by setting expectations more realistically, 
| e.g. M2 in the example above Q1.5 would understand their relationship with 
| M1 and would understand better why things might get looked at again.
| Compared with today where both being in MAINTAINERS gives M2 the
| expectation that they are equal to M1, which makes M2 frustrated when 
| M1 appears to undermine them.

| I think it would, yes. It's of course hard to tell in advance, but the
| reason why I think it would be better is simply that it would make
| things more clear. 

| That would set the right expectation, but can't necessarily help improve
| trust. Active committers have very high standards : in other words 
| building trust to the level they require need a lot of investment.For 
| people who are not working on open source project full time (or most of 
| their time) it would be very hard.

| I don't think a formal maintainer hierarchy is the only way to fix
| this. But certainly expectations need to be readjusted.

| Yes, having a more clear distinction would help, but there's still the
| problem of nested maintainership, and even if that's written down, I'm
| not sure it's going to help erase the tensions between
| maintainers/committers and submaintainers.

! It appears to me that the idea of nested ownership, would maybe be clearer
! and better set expectations. It is also consistent with the expectation
! of "meritocracy". This may in turn may better set expectations
! for contributors. It is unclear, whether we need to change governance at 
! this stage: maybe a good starting point would be to work together on
! "A guide (or best practices) for maintainers" and maybe clarify some things
! in the MAINTAINERS file in parallel. We could also work together on a "A 
! guide  (or best practices) for committers". It may well turn out, that the 
! latter is slim, if we can separate the hats committers wear more clearly. 
! If then there are discrepancies, we can still change the governance, if 
! needed. 

! I think some of the issues raised by people feeling unhappy with the status
! quo may be because of mismatched expectations compounded by other issues. I 
! think there is some truth in the statement "the problems you are describing 
! seem to be to to with (a) nested maintainership, (b) maintainers disagreeing, 
! or (c) slow review, and not the committer's role". We may also have some
! clashing personalities: but these always exist in any organisation. 

! On the review bottleneck in general, I do have some initial data and should 
! get a more detailed report shortly. But there are two key observations from
! the initial report:
! 
! = Double the number of Contributions since 2012 =
! * the amount of patches submitted per month has gone up linearly up from 
!   around 115 in 2012 to about 230
! = Growth of Number of Review comments by an order of magnitude =
! * the amount of review comments per month has stayed constant till 2012
! * from 2012 we saw a linear increase on of the amount of review comments.
!   It is now 7-8x higher compared to 2012
! * the mean number of comments per patch was around 5.5 until 2012 and has
!   linearly grown to 16
! * the median number of comments per patch was around 2.5 until 2012 and has
!   linearly grown to 5.5
! = Exponential growth of waiting time to review =
! * the mean time to review has grown exponentially from this year
!   (a clear sign of a bottleneck and stress we hit this year)
!
! What is clear is that not just contribution growth is responsible
! for the bottleneck. It seems that we have *significantly* higher expectations
! on quality today, leading to more comments and taking up more time. 
! The two things together are causing the stress.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel