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

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

[Lars Kurth]

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@lists.xen.org
http://lists.xen.org/xen-devel

Re: Survey Results - Part 1 : Improving Xen Project Governance and Conventions

[Jan Beulich]

>>> On 05.10.15 at 13:27, <lars.kurth.xen@gmail.com> wrote:
> | 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'd be curious of examples of this.

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

And quite a bit more so for this.

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

Agreed, but without being more specific about the issues (which
may make it unavoidable to name names) I don't see a way forward.

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

Well, we already have ways to express nested vs non-nested
maintainership in ./MAINTAINERS - via the X: prefix. Maybe we
should make broader use of it. Despite seeing all the responses
and comments I still think that (at least as far as I'm concerned)
maintainers are equal, i.e. a more narrow scope maintainer's ack
is sufficient for a change to go in, regardless of X: use. Of course
that doesn't rule out the wider scope maintainer requesting
changes despite the other's ack, but that's symmetrical: For a
change to go in, no maintainer should disagree (and in fact
reasonable disagreement by a non-maintainer counts too).

By formalizing nested maintainership and requiring ack-s from
each one in the hierarchy we will just further slow down things.
If a maintainer doesn't trust a sub-maintainer, the question
needs to be raised whether the sub-maintainer was nominated
prematurely, or whether conditions changed after nomination
that warrant reverting that state.

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

Which is a result from the quality issues we inherited from the time
when patches got committed without much review. The amount of
security issues is only one indicator. When I write code for a new
feature (which is rare enough) or review contributions, I pay close
attention to the code modified or needing taking into consideration,
frequently leading to me spotting bugs unrelated to the actual work
I do. While other committers and maintainers do so too afaict, the
majority of contributors act as if existing code is guaranteed bug
free; I've even seen them copy'n'paste buggy code.

The original model was certainly not unreasonable as long as Xen
was still more of a university/research project, but the transition
to one intended for wide production use should have happened
much earlier.

And, not to forget, the higher quality requirements also have a
connection to the growing size of the project, and the need to
keep the overall thing maintainable.

Jan

Re: Survey Results - Part 1 : Improving Xen Project Governance and Conventions

[Lars Kurth]

> On 5 Oct 2015, at 15:32, Jan Beulich <jbeulich@suse.com> wrote:
> 
>> ! 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.
> 
> Agreed, but without being more specific about the issues (which
> may make it unavoidable to name names) I don't see a way forward.

Maybe the best approach would be for people to come forward with examples. But I can't make that decision for them.

Alternatively, the code review data may provide some more insights when done. I shared what I have for now in a separate thread. The plan is to have a more detailed report for the next Advisory Board on Oct the 20th, with an analysis / executive summary. We can then look into investigation areas and/or create tooling that allows us to do regular reporting.

>> ! 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. 
> 
> Well, we already have ways to express nested vs non-nested
> maintainership in ./MAINTAINERS - via the X: prefix. Maybe we
> should make broader use of it. Despite seeing all the responses
> and comments I still think that (at least as far as I'm concerned)
> maintainers are equal, i.e. a more narrow scope maintainer's ack
> is sufficient for a change to go in, regardless of X: use.

I don't think we were ever really clear what whose ACK is or should be necessary and/or sufficient. 

I think that that notion would frame the concept of a hierarchy more clealry: it would be good to understand what happens in practice today and whether current practice is consistent across the community.

> Of course
> that doesn't rule out the wider scope maintainer requesting
> changes despite the other's ack, but that's symmetrical: For a
> change to go in, no maintainer should disagree (and in fact
> reasonable disagreement by a non-maintainer counts too).

Isn't that merely replacing an active ACK with an objection. Of course for this model to work well, we would need the equivalent of a time-out for an objection. But then you could also say that no active ACK given by the time-out period counts like an ACK. But then you guys deal with protocols of this kind all the time and should know what is best.

In fact some of the feedback from the survey suggests that the disagreements that are highlighted after some time has passed cause the most grievances. 

> By formalizing nested maintainership and requiring ack-s from
> each one in the hierarchy we will just further slow down things.
> If a maintainer doesn't trust a sub-maintainer, the question
> needs to be raised whether the sub-maintainer was nominated
> prematurely, or whether conditions changed after nomination
> that warrant reverting that state.

That is a very valid point: some of this was also raised in another part of the survey (part 2, which tries to explore whether maybe there are trust issues).

I am not sure whether the hierarchy would require everyone to ACK. As far as I understand Linux does this without (but I suppose the ACK is replaced by a pull request the next level up the hierarchy, which sort of is like one).  

>> ! 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. 
> 
> Which is a result from the quality issues we inherited from the time
> when patches got committed without much review. The amount of
> security issues is only one indicator. When I write code for a new
> feature (which is rare enough) or review contributions, I pay close
> attention to the code modified or needing taking into consideration,
> frequently leading to me spotting bugs unrelated to the actual work
> I do. While other committers and maintainers do so too afaict, the
> majority of contributors act as if existing code is guaranteed bug
> free; I've even seen them copy'n'paste buggy code.
> 
> The original model was certainly not unreasonable as long as Xen
> was still more of a university/research project, but the transition
> to one intended for wide production use should have happened
> much earlier.
> 
> And, not to forget, the higher quality requirements also have a
> connection to the growing size of the project, and the need to
> keep the overall thing maintainable.

Not disagreeing. I don't think we ever had a discussion about this trade-off: it sort of evolved over time, without being discussed explicitly. That may well be the cause for a mismatch of expectations. 

I also think that for some areas of code (e.g. if experimental and sufficiently modular) a more relaxed approach may well be acceptable until it is more widely adopted. Maybe we can also link this to the "Feature Lifecycle Maturity" proposal I made a while back.

Regards 
Lars

Re: Survey Results - Part 1 : Improving Xen Project Governance and Conventions

[George Dunlap]

On Mon, Oct 5, 2015 at 11:31 PM, Lars Kurth <lars.kurth.xen@gmail.com> wrote:
>
>> On 5 Oct 2015, at 15:32, Jan Beulich <jbeulich@suse.com> wrote:
>>
>>> ! 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.
>>
>> Agreed, but without being more specific about the issues (which
>> may make it unavoidable to name names) I don't see a way forward.
>
> Maybe the best approach would be for people to come forward with examples. But I can't make that decision for them.

My hope is that just having aired some of the concerns will help
somewhat. People who have been silently frustrated with the status quo
should now know that they aren't the only ones; and should be
encouraged to bring up examples the next time they come up.  Hopefully
also committers will be more aware of the effects of giving high
degrees of duplicate review, and be more thoughtful about when they do
it.

> I also think that for some areas of code (e.g. if experimental and sufficiently modular) a more relaxed approach may well be acceptable until it is more widely adopted. Maybe we can also link this to the "Feature Lifecycle Maturity" proposal I made a while back.

FWIW we do sort of have this already: the arinc scheduler, for
instance, was completely self-contained, and so went in with actually
very little review of the scheduling code itself.

Unfortunately, there are very few features which can be isolated to
this degree.  The vast majority of features submitted recently touch
core code which is used frequently (and so can't be isolated); and
also includes a public interface, which is something that we have to
be very careful to get right, because we are committed to supporting
it for years.

 -George

Re: Survey Results - Part 1 : Improving Xen Project Governance and Conventions

[Ian Campbell]

On Tue, 2015-10-06 at 10:19 +0100, George Dunlap wrote:
> On Mon, Oct 5, 2015 at 11:31 PM, Lars Kurth <lars.kurth.xen@gmail.com>
> > I also think that for some areas of code (e.g. if experimental and
> > sufficiently modular) a more relaxed approach may well be acceptable
> > until it is more widely adopted. Maybe we can also link this to the
> > "Feature Lifecycle Maturity" proposal I made a while back.
> 
> FWIW we do sort of have this already: the arinc scheduler, for
> instance, was completely self-contained, and so went in with actually
> very little review of the scheduling code itself.
> 
> Unfortunately, there are very few features which can be isolated to
> this degree.  The vast majority of features submitted recently touch
> core code which is used frequently (and so can't be isolated); and
> also includes a public interface, which is something that we have to
> be very careful to get right, because we are committed to supporting
> it for years.

Also we should be careful that experimental features accepted in a more
relaxed way actually do get checked per the usual requirements (whether the
same as today or modified) before being declared something more than
experimental (whether that is production or some intermediate state).

I think we have had instances of this in the past which has lead to issues
(e.g. security ones or maintenance ones) down the line, although I have to
confess I'm drawing a blank on examples right now.

That's something where a proposal like Lars' lifecycle thing would help,
but I would worry that once the code is in tree it would't get the same
level of scrutiny when it comes to get "upgraded" as it would if it were
being submitted as a non-experimental feature in the first place, even we
say it should. I think simply being aware of that possibility is probably
sufficient to render this a non-blocker if we want to try this approach
though.

Ian.

Re: Survey Results - Part 1 : Improving Xen Project Governance and Conventions

[Jan Beulich]

>>> On 06.10.15 at 11:36, <ian.campbell@citrix.com> wrote:
> On Tue, 2015-10-06 at 10:19 +0100, George Dunlap wrote:
>> On Mon, Oct 5, 2015 at 11:31 PM, Lars Kurth <lars.kurth.xen@gmail.com>
>> > I also think that for some areas of code (e.g. if experimental and
>> > sufficiently modular) a more relaxed approach may well be acceptable
>> > until it is more widely adopted. Maybe we can also link this to the
>> > "Feature Lifecycle Maturity" proposal I made a while back.
>> 
>> FWIW we do sort of have this already: the arinc scheduler, for
>> instance, was completely self-contained, and so went in with actually
>> very little review of the scheduling code itself.
>> 
>> Unfortunately, there are very few features which can be isolated to
>> this degree.  The vast majority of features submitted recently touch
>> core code which is used frequently (and so can't be isolated); and
>> also includes a public interface, which is something that we have to
>> be very careful to get right, because we are committed to supporting
>> it for years.
> 
> Also we should be careful that experimental features accepted in a more
> relaxed way actually do get checked per the usual requirements (whether the
> same as today or modified) before being declared something more than
> experimental (whether that is production or some intermediate state).
> 
> I think we have had instances of this in the past which has lead to issues
> (e.g. security ones or maintenance ones) down the line, although I have to
> confess I'm drawing a blank on examples right now.

I think tmem would make a very good one.

Jan

Re: Survey Results - Part 1 : Improving Xen Project Governance and Conventions

[Lars Kurth]

> On 6 Oct 2015, at 10:19, George Dunlap <dunlapg@umich.edu> wrote:
> 
> On Mon, Oct 5, 2015 at 11:31 PM, Lars Kurth <lars.kurth.xen@gmail.com> wrote:
>> 
>>> On 5 Oct 2015, at 15:32, Jan Beulich <jbeulich@suse.com> wrote:
>>> 
>>>> ! 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.
>>> 
>>> Agreed, but without being more specific about the issues (which
>>> may make it unavoidable to name names) I don't see a way forward.
>> 
>> Maybe the best approach would be for people to come forward with examples. But I can't make that decision for them.
> 
> My hope is that just having aired some of the concerns will help
> somewhat. People who have been silently frustrated with the status quo
> should now know that they aren't the only ones; and should be
> encouraged to bring up examples the next time they come up.  Hopefully
> also committers will be more aware of the effects of giving high
> degrees of duplicate review, and be more thoughtful about when they do
> it.

Agreed.

>> I also think that for some areas of code (e.g. if experimental and sufficiently modular) a more relaxed approach may well be acceptable until it is more widely adopted. Maybe we can also link this to the "Feature Lifecycle Maturity" proposal I made a while back.
> 
> FWIW we do sort of have this already: the arinc scheduler, for
> instance, was completely self-contained, and so went in with actually
> very little review of the scheduling code itself.
> 
> Unfortunately, there are very few features which can be isolated to
> this degree.  The vast majority of features submitted recently touch
> core code which is used frequently (and so can't be isolated); and
> also includes a public interface, which is something that we have to
> be very careful to get right, because we are committed to supporting
> it for years.

I did not fully understand that this was the case: my hope was that core functionality may be more modular than it actually is. 

Lars 

Re: Survey Results - Part 1 : Improving Xen Project Governance and Conventions

[Ian Campbell]

On Mon, 2015-10-05 at 23:31 +0100, Lars Kurth wrote:
> I am not sure whether the hierarchy would require everyone to ACK. As far
> as I understand Linux does this without (but I suppose the ACK is
> replaced by a pull request the next level up the hierarchy, which sort of
> is like one).  

That's my understanding, by forwarding it to the next level a maintainer is
in effect acking it (in fact they would normally add a S-o-b per 3(c) of
the DCO).

Ian.

Re: Survey Results - Part 1 : Improving Xen Project Governance and Conventions

[Lars Kurth]

> On 6 Oct 2015, at 10:29, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> 
> On Mon, 2015-10-05 at 23:31 +0100, Lars Kurth wrote:
>> I am not sure whether the hierarchy would require everyone to ACK. As far
>> as I understand Linux does this without (but I suppose the ACK is
>> replaced by a pull request the next level up the hierarchy, which sort of
>> is like one).  
> 
> That's my understanding, by forwarding it to the next level a maintainer is
> in effect acking it (in fact they would normally add a S-o-b per 3(c) of
> the DCO).

Interesting. It appears to me that in Linux the hierarchy fulfils two functions: 
a) it is a reflection of seniority 
b) it also acts as a queuing system of patches 

So basically, once a patch or a series in Linux is ACKED at the bottom of the tree, it just travels up the tree until it hits Linus. There are several hand-overs along the way, which either involve a re-review, scan, pass-through based on how much the source is trusted. As a contributor, you mainly deal with the bottom of the hierarchy and don't have to think much about what happens further up the chain. 

If you contrast this with our model: we basically have a situation where we have one big queue (xen-devel). And each maintainer has their own queue via the CC field and then manages their own queue. But the actual review and ACKs happen on xen-devel. A review is not complete until all stake-holders (maintainers and/or committers which are also maintainers) signalled they are happy. 

Committers check whether all ACKs are there (or sufficient) and also spend some time doing some manual coordination, to make sure that missing ACKs and reviews for series that are close, get completed.

If we consider, some of the survey data on back-logs, e.g.
- Active (aka activity in the last 7 days) : 78
- Ongoing (aka activity in the last 12 months): 403
it is probably fair enough to assume that at any given time there are somewhere between 50-250 series undergoing reviews (assuming that the figures from the study are too high). 

This makes it likely that for complex series we just hit scalability issues with this "flat" approach. If you assume
- ACKs from a range of different maintainers are required
- Maintainers use different strategies in prioritising what to look at
- The large number of reviews that are ongoing
statistically it would take longer to complete a review, if the number of series undergoing reviews was much smaller. 

It's kind of like shooting paint balls (not randomly, but with some degree of randomness) at pieces of paper that are fixed to wall and declaring a piece done, when it is covered with paint and the "inspector" (committer) declares it's covered well enough. At that point, the piece of paper is removed. The bigger the wall, the longer the process takes.

I recall, that someone said that the Linux model couldn't work for us, because the code structure can't be as easily mapped into the tree model. And we also don't want to change the work-flow significantly, as most maintainers seem to be happy with it.

I wonder, whether there is something else we can do to focus the attention of "paint guns" (sorry for the analogy) at smaller portions of the wall more synchronously. I have not really thought this through and just wanted to put this out as an idea. Assuming that we can fix the mapping issues with incomplete reviews from the tools, it seems conceivable that we can define some buckets of ongoing reviews (or section the "wall") and serve them up as web pages to influence what reviews maintainers prioritise.

Regards
Lars

Re: Survey Results - Part 1 : Improving Xen Project Governance and Conventions

[Ian Campbell]

On Fri, 2015-10-16 at 11:33 +0100, Lars Kurth wrote:
> > On 6 Oct 2015, at 10:29, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > 
> > On Mon, 2015-10-05 at 23:31 +0100, Lars Kurth wrote:
> > > I am not sure whether the hierarchy would require everyone to ACK. As
> > > far
> > > as I understand Linux does this without (but I suppose the ACK is
> > > replaced by a pull request the next level up the hierarchy, which
> > > sort of
> > > is like one).  
> > 
> > That's my understanding, by forwarding it to the next level a
> > maintainer is
> > in effect acking it (in fact they would normally add a S-o-b per 3(c)
> > of
> > the DCO).
> 
> Interesting. It appears to me that in Linux the hierarchy fulfils two
> functions: 
> a) it is a reflection of seniority 

More of increasing size of "umbrella of influence/authority". Which very
likely does correlate strongly with seniority though, true.

> b) it also acts as a queuing system of patches 
> 
> So basically, once a patch or a series in Linux is ACKED at the bottom of
> the tree, it just travels up the tree until it hits Linus. There are
> several hand-overs along the way, which either involve a re-review, scan,
> pass-through based on how much the source is trusted. As a contributor,
> you mainly deal with the bottom of the hierarchy and don't have to think
> much about what happens further up the chain. 
> 
> If you contrast this with our model: we basically have a situation where
> we have one big queue (xen-devel). And each maintainer has their own
> queue via the CC field and then manages their own queue. But the actual
> review and ACKs happen on xen-devel. A review is not complete until all
> stake-holders (maintainers and/or committers which are also maintainers)
> signalled they are happy. 
>
> Committers check whether all ACKs are there (or sufficient) and also
> spend some time doing some manual coordination, to make sure that missing
> ACKs and reviews for series that are close, get completed.

Sounds about right.

> If we consider, some of the survey data on back-logs, e.g.
> - Active (aka activity in the last 7 days) : 78
> - Ongoing (aka activity in the last 12 months): 403
> it is probably fair enough to assume that at any given time there are
> somewhere between 50-250 series undergoing reviews (assuming that the
> figures from the study are too high). 

FWIW, completely anecdotally, my QUEUE-4.7 mail folder, which where I am
currently dragging any incoming patch series which I think might at some
point require attention currently has 101 distinct threads in it, some of
which are resends etc. 

If I commit something I drag all the associated threads into my genericxen-devel folder). I do occasional gardening of the folder too, but there
will be some amount of stuff which has been committed by someone else or
which is otherwise "done" still sitting in there.

So I would guess that I currently have maybe 100/N series currently "on my
radar" in one way or another, where N would be the average resend count
(some smallish integer I would guess).

Also as some anecdotal data about abandonment, my old QUEUE-4.6 folder
still has 90 threads in it. I switch folder around the time of the freeze
(and queue things which might be subject to a freeze exception in the old
one etc) so those are either things which I didn't spot being committed,
are abandoned or which have been reposted with the repost going into QUEUE
-4.7 and I've forgotten to retrieve the old version from QUEUE-4.6 into
QUEUE-4.7 (which I normally try to do). I'm not quite as diligent about
gardening the non-current folders though, so assume more cruft has
accumulated.

(Aside: I was thinking recently about a tool which given a Maildir and one
or more git trees would say "this looks like it might have been committed"
so I can do less gardening, this is not a request for anyone to write such
a tool though ;-))

> This makes it likely that for complex series we just hit scalability
> issues with this "flat" approach. If you assume
> - ACKs from a range of different maintainers are required
> - Maintainers use different strategies in prioritising what to look at
> - The large number of reviews that are ongoing
> statistically it would take longer to complete a review, if the number of
> series undergoing reviews was much smaller. 
> 
> It's kind of like shooting paint balls (not randomly, but with some
> degree of randomness) at pieces of paper that are fixed to wall and
> declaring a piece done, when it is covered with paint and the "inspector"
> (committer) declares it's covered well enough. At that point, the piece
> of paper is removed. The bigger the wall, the longer the process takes.

There's some truth in that analogy I think ;-)

> I recall, that someone said that the Linux model couldn't work for us,
> because the code structure can't be as easily mapped into the tree model.

FWIW that's the case for some aspects of Linux too.

e.g. linux/arch/*/xen is sort of considered part of the "Xen subsystem"
along with e.g. linux/drivers/xen and not part of the linux/arch/{x86,arm}
subsystem.

Patches are still posted to both but AIUI the relevant maintainers have
arrived at a mutual understanding that stuff which doesn't stray outside of
the arch/*/xen can generally go in via the xen tree without an ack from the
arch maintainers and that the maintainers are trusted to use their
judgement as to when this might not be appropriate and not abuse it etc.

Which ultimately just comes down to allowing maintainers to use their
judgement and to talk to each other to come to a mutual understanding etc,
rather than being forced to arbitrarily follow the filesystem (or any
other) hierarchy.

> I wonder, whether there is something else we can do to focus the
> attention of "paint guns" (sorry for the analogy) at smaller portions of
> the wall more synchronously. I have not really thought this through and
> just wanted to put this out as an idea. Assuming that we can fix the
> mapping issues with incomplete reviews from the tools, it seems
> conceivable that we can define some buckets of ongoing reviews (or
> section the "wall") and serve them up as web pages to influence what
> reviews maintainers prioritise.

If such a thing could be done without too many false positives and without
too much need for manually poking things by maintainers then I can see that
being a valuable, yes.

I can imagine that even just a list of "in fight" series without any sort
of mapping to maintainers or tracking of which acks it still needs etc
would potentially be valuable (won't really know till we have it though).

Ian.

Re: Survey Results - Part 1 : Improving Xen Project Governance and Conventions

[Jan Beulich]

>>> On 06.10.15 at 00:31, <lars.kurth.xen@gmail.com> wrote:
>> On 5 Oct 2015, at 15:32, Jan Beulich <jbeulich@suse.com> wrote:
>> Of course
>> that doesn't rule out the wider scope maintainer requesting
>> changes despite the other's ack, but that's symmetrical: For a
>> change to go in, no maintainer should disagree (and in fact
>> reasonable disagreement by a non-maintainer counts too).
> 
> Isn't that merely replacing an active ACK with an objection. Of course for 
> this model to work well, we would need the equivalent of a time-out for an 
> objection. But then you could also say that no active ACK given by the 
> time-out period counts like an ACK. But then you guys deal with protocols of 
> this kind all the time and should know what is best.

Keir has always been advocating the "we can always revert", and
I support this model. Indeed there have been a few cases were
things got reverted because of late feedback.

> In fact some of the feedback from the survey suggests that the disagreements 
> that are highlighted after some time has passed cause the most grievances. 

I'm not sure I understand: How is late disagreement any worse
than immediate one?

>>> ! 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. 
>> 
>> Which is a result from the quality issues we inherited from the time
>> when patches got committed without much review. The amount of
>> security issues is only one indicator. When I write code for a new
>> feature (which is rare enough) or review contributions, I pay close
>> attention to the code modified or needing taking into consideration,
>> frequently leading to me spotting bugs unrelated to the actual work
>> I do. While other committers and maintainers do so too afaict, the
>> majority of contributors act as if existing code is guaranteed bug
>> free; I've even seen them copy'n'paste buggy code.
>> 
>> The original model was certainly not unreasonable as long as Xen
>> was still more of a university/research project, but the transition
>> to one intended for wide production use should have happened
>> much earlier.
>> 
>> And, not to forget, the higher quality requirements also have a
>> connection to the growing size of the project, and the need to
>> keep the overall thing maintainable.
> 
> Not disagreeing. I don't think we ever had a discussion about this trade-off: 
> it sort of evolved over time, without being discussed explicitly. That may 
> well be the cause for a mismatch of expectations. 
> 
> I also think that for some areas of code (e.g. if experimental and 
> sufficiently modular) a more relaxed approach may well be acceptable until it 
> is more widely adopted. Maybe we can also link this to the "Feature Lifecycle 
> Maturity" proposal I made a while back.

As already kind of pointed out by Ian, doing so would lead to the
same problems we had and in some areas still have: Code added
with at best lax review gets named supported after a while
(perhaps simply on the basis of no problem sightings), and issues
found subsequently (which would likely have been found by proper
initial review) then end up being severe. Plus - the smaller the
feature, the more likely that we end up without any explicit
support statement, making the thing implicitly supported in at least
some people's eyes. It has been more than once that on the
security team we had a hard time figuring whether we could waive
the security aspect of a report because the affected code would
_seem_ to be unsupported.

Jan

Re: Survey Results - Part 1 : Improving Xen Project Governance and Conventions

[Stefano Stabellini]

On Tue, 6 Oct 2015, Jan Beulich wrote:
> > In fact some of the feedback from the survey suggests that the disagreements 
> > that are highlighted after some time has passed cause the most grievances. 
> 
> I'm not sure I understand: How is late disagreement any worse
> than immediate one?

It is much harder (psychologically if nothing else) to revisit a patch
1-2 months after it was written, compared to 1-2 days after it was
written.

Re: Survey Results - Part 1 : Improving Xen Project Governance and Conventions

[Ian Campbell]

On Mon, 2015-10-05 at 08:32 -0600, Jan Beulich wrote:

> > ! 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. 
> 
> Well, we already have ways to express nested vs non-nested
> maintainership in ./MAINTAINERS - via the X: prefix. Maybe we
> should make broader use of it. Despite seeing all the responses
> and comments I still think that (at least as far as I'm concerned)
> maintainers are equal, i.e. a more narrow scope maintainer's ack
> is sufficient for a change to go in, regardless of X: use. Of course
> that doesn't rule out the wider scope maintainer requesting
> changes despite the other's ack, but that's symmetrical: For a
> change to go in, no maintainer should disagree (and in fact
> reasonable disagreement by a non-maintainer counts too).
> 
> By formalizing nested maintainership and requiring ack-s from
> each one in the hierarchy we will just further slow down things.
> If a maintainer doesn't trust a sub-maintainer, the question
> needs to be raised whether the sub-maintainer was nominated
> prematurely, or whether conditions changed after nomination
> that warrant reverting that state.

I don't think it is a matter of trust, just that the support for some
feature/subsystem is just one part of a larger whole and while the
maintainer of that feature might be entirely trusted/empowered WRT that
area it still interacts with the larger whole. Unfortunately this is not
always expressible at a file granularity.

Taking libxl as an example, the are certainly features in there which have
a specific maintainer whose ack is all that is needed WRT that feature, but
some patches also need to be considered in light of the larger whole of the
entire library e.g things like:

 * API consistency
    * consistent function naming in the public API
    * common argument patterns
    * use of error codes
    * not breaking API stability
    * suitability for on going support as a stable API
 * Coding Style issues
    * the error handling idiom
    * memory allocation idiom
 * Interactions with the rest of the library
    * the asynchronous operations framework
    * ...

Now arguably any maintainer of a piece of libxl ought to be familiar with
all of that. But, personally I don't think that is very reasonable either
as a burden for those feature maintainers or particularly as a strategy for
ensuring the library remains coherent.

I included "the asynchronous operations framework" deliberately because it
is a quite complex thing which I don't think we can expect all maintainers
who support code within libxl to be intimately familiar with.

Even for the more "every day" stuff I listed above I think it is not
uncommon that the majority of the complexity for some feature is in the
hypervisor, but that it needs exposing up through the toolstack. The
maintainer of that feature mayor may not own it all the way up to the
toolstack level but it would be quite reasonable for them to do so, but
they still may not be completely up to speed on all the above.

Some care is needed on behalf of the "library reviewers" in terms of not
stepping on the toes of the feature maintainer wrt code which is only
relevant to the feature and/or making it clear when one is raising an issue
with that hat vs. just being another 3rd party who the feature maintainer
is free to ignore. This conversation has certainly made me more aware of
this.

Ian.

Re: Survey Results - Part 1 : Improving Xen Project Governance and Conventions

[Jan Beulich]

>>> On 06.10.15 at 12:26, <ian.campbell@citrix.com> wrote:
> Now arguably any maintainer of a piece of libxl ought to be familiar with
> all of that. But, personally I don't think that is very reasonable either
> as a burden for those feature maintainers or particularly as a strategy for
> ensuring the library remains coherent.

Agreed. But rather than always requiring multiple acks, wouldn't it be
more reasonable for the "small" maintainer to explicitly request an ack
from the "large" one in such cases?

Jan

Re: Survey Results - Part 1 : Improving Xen Project Governance and Conventions

[Ian Campbell]

On Tue, 2015-10-06 at 06:25 -0600, Jan Beulich wrote:
> > > > On 06.10.15 at 12:26, <ian.campbell@citrix.com> wrote:
> > Now arguably any maintainer of a piece of libxl ought to be familiar with
> > all of that. But, personally I don't think that is very reasonable either
> > as a burden for those feature maintainers or particularly as a strategy for
> > ensuring the library remains coherent.
> 
> Agreed. But rather than always requiring multiple acks, wouldn't it be
> more reasonable for the "small" maintainer to explicitly request an ack
> from the "large" one in such cases?

I suppose it requires the feature maintainer to be more aware of the
library level stuff, i.e. aware enough of those issues to know when to ask.

But that's a bit of a lame answer and I think it could reasonably be
expected, so it's not all that different I guess.

I'm not sure what proportion of feature patches to libxl would require
taking this "trap" to the library maintainers, my gut says it would be that
it is more often than not.

Ian.

Re: Survey Results - Part 1 : Improving Xen Project Governance and Conventions

[Lars Kurth]

> On 6 Oct 2015, at 11:26, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> 
> On Mon, 2015-10-05 at 08:32 -0600, Jan Beulich wrote:
> 
>> By formalizing nested maintainership and requiring ack-s from
>> each one in the hierarchy we will just further slow down things.
>> If a maintainer doesn't trust a sub-maintainer, the question
>> needs to be raised whether the sub-maintainer was nominated
>> prematurely, or whether conditions changed after nomination
>> that warrant reverting that state.
> 
> I don't think it is a matter of trust, just that the support for some
> feature/subsystem is just one part of a larger whole and while the
> maintainer of that feature might be entirely trusted/empowered WRT that
> area it still interacts with the larger whole. Unfortunately this is not
> always expressible at a file granularity.
> 
> Taking libxl as an example, the are certainly features in there which have
> a specific maintainer whose ack is all that is needed WRT that feature, but
> some patches also need to be considered in light of the larger whole of the
> entire library e.g things like:
> 
> * API consistency
>    * consistent function naming in the public API
>    * common argument patterns
>    * use of error codes
>    * not breaking API stability
>    * suitability for on going support as a stable API
> * Coding Style issues
>    * the error handling idiom
>    * memory allocation idiom
> * Interactions with the rest of the library
>    * the asynchronous operations framework
>    * ...
> 
> Now arguably any maintainer of a piece of libxl ought to be familiar with
> all of that. But, personally I don't think that is very reasonable either
> as a burden for those feature maintainers or particularly as a strategy for
> ensuring the library remains coherent.
> 
> I included "the asynchronous operations framework" deliberately because it
> is a quite complex thing which I don't think we can expect all maintainers
> who support code within libxl to be intimately familiar with.
> 
> Even for the more "every day" stuff I listed above I think it is not
> uncommon that the majority of the complexity for some feature is in the
> hypervisor, but that it needs exposing up through the toolstack. The
> maintainer of that feature mayor may not own it all the way up to the
> toolstack level but it would be quite reasonable for them to do so, but
> they still may not be completely up to speed on all the above.
> 
> Some care is needed on behalf of the "library reviewers" in terms of not
> stepping on the toes of the feature maintainer wrt code which is only
> relevant to the feature and/or making it clear when one is raising an issue
> with that hat vs. just being another 3rd party who the feature maintainer
> is free to ignore. This conversation has certainly made me more aware of
> this.

I am not sure whether this is really that different to other projects. In many ways what you listed is "architectural and stylistic coherence", which is something you'd expect someone more experienced to pick up, which in this case would be the libxl maintainer (or committer). Or maybe we can divide this up more clearly amongst co-maintainers for libxl, to build in some redundancy.

Or us something like "X: libel error handling idiom ... person" in MAINTAINERS

Lars