[discussion] who should validate a decision recording PR?

[nweldev]

As we discussed during today's Openness Meeting, we'll need to define more clearly who should be responsible of merging (i.e. accept) a PR request adding a new decision record, or modifying an existing one.

First, we'll just try to have every staff member on board with the decision, even if it only means putting a "LGTM" (Looks Good To Me) comment. Then, we'll see as we go and discuss here how we could be more efficient.

Stuff to discuss

• What is the exact role of a "Decision Record Sponsor"?
• Could it be a "meritocratic" process?
• Should we add "contributors" and "reviews" sections to the template's headers?

[nweldev]

Proposal: sponsoring and review process

• Before merging, any decision recording PR requires to define:
  • Author(s): one or more community member who writes the decision record and is committed to championing it through the process
  • Sponsor: a single member of the bindnet organization with a @blindnet.io email address who shepherds the decision record through the review process
  • Reviewer(s): one or more member of the bindnet organization with a @blindnet.io email address who has the responsibility of recommending the adoption of the decision or its rejection
• a decision's author CAN also act as the decision sponsor IF AND ONLY IF he or she is a member of the bindnet organization with a @blindnet.io email address AND an active contributor to the associated project or the openness framework
• a decision's author CAN NOT be a reviewer for the same decision
• a decision's sponsor CAN NOT be a reviewer for the same decision
• Sponsor, author(s) and reviewer(s) SHOULD be clearly defined in a markdown header following the decision framework templates
• ONLY a reviewer can merge the PR

[nweldev]

ping @blindnet-io/blindnet to validate this ☝️

[milstan]

LGTM

[filipblnt]

After a while in this I am thinking a bit about sponsors' practical roles. For example, in a PR here we had the same person as a reviewer and assignee, and the same person was also a sponsor of the document that was under review.
So, in a situation in which a sponsor is neither an author not reviewer (which according to the proposal seems realistic), where do you see sponsor's contribution? I am not at all sure about this, seems to me now that sponsors are not really necessary and that we could only have authors/contributors, but let's challenge that.

Adding contributors and reviewers section to the header seems like a good idea.

[milstan]

Realistically, I think we should aim for a lower number of (mutually exclusive) roles, having mind the team size.

Maybe what I seem to understand now, is that Author can be a Sponsor? In which case, I've made a mistake to put you @filipblnt as sponsor where your should have been a reviewer?

[filipblnt]

Realistically, I think we should aim for a lower number of (mutually exclusive) roles, having mind the team size.

Totally agree.

Maybe what I seem to understand now, is that Author can be a Sponsor? In which case, I've made a mistake to put you @filipblnt as sponsor where your should have been a reviewer?

Maybe, but I am not sure yet as I don't have a clear picture of what would have been my role as a sponsor there, and so far I have only acted as a reviewer. This is why I've put my initial comment.

[milstan]

So concretely, what happens now with my pull request that I am so desperately waiting to get merged.

I've got one review (out of 1 that I requested). I assigned Noël, who I also asked for review (not clear if this is expected behavior - maybe we should clarify)

I think our procedure here is not sufficient as it is mostly about what SHOULD NOT happen, and very little about what SHOULD/MUST happen.

I think we need further clarity about:

• who asks for review
• favor explicit review requests rather than "everybody should"
• make sure there is a clearly designated person who should decide (a positive definition - not a definition of who should not)

[nweldev]

Scope of this issue

So, there are two parts to this discussion I think:

1. decision records, which follow the decision framework, which applies to all projects but only for specific document types as defined in DecisionFramework > what-template-will-you-use
2. other types of documents (like More on Software Systems + Restructuring product-management#616 and Architecture proposal for v2 of Privateform product-management#596), with a more permissive workflow or, for some cases (like user stories and specs) a dedicated workflow that depends on the actual project

Here, this issue, being part of our openness framework, is only about (1) IMHO (more about that at the end of this comment)

Clarifying what "sponsor" means, and why we need one

To answer @filipblnt #14 (comment): having a sponsor is mostly about helping outside contributions. Right now, as all decisions are made internally without much contribution from the community. So you're right, we don't need a separate sponsor here, and this is why, in this context, author = sponsor. In the future, when an outsider (the author) writes an RFC, for example, a sponsor will be defined to accompany them through the whole process, and make sure staff members actually help and contribute.

So, to put it simply: the Sponsor is the one person responsible for making sure

1. (during the decision making process / PR review) the author is actually heard, the proposal considered, and the process respected
2. (after the decision has been recorded / PR merge) the decision is applied, respected and understood (i.e. if someone has a question about this decision, they should reach to the sponsor)

In your example, Milan, as the de-facto author, being a blindnet staff member, should have been de-facto sponsor according to #14 (comment)

Limiting the number of roles

About @milstan #14 (comment) « we should aim for a lower number of (mutually exclusive) roles »: Github itself is enough, IMHO, to track contributors and reviewers. We don't want this to be duplicated. This is why I didn't "officially" define specific roles for them, and didn't put them in the document's header. Having just (ideally one) "official/lead" author(s) and reviewer(s) in the header is enough. We don't need to track all of them. And yes, when the decision record is written by a blindnet staff member, author = sponsor.

SHOULD/MUST

#14 (comment)

(2 - other types of documents) could also be about code or documentation, so it depends a lot on the specific type of document and specific project we use. It doesn't require formally defining a sponsor. Instead, it should, IMHO, be addressed in blindnet-io/devrel-management#20, blindnet-io/devrel-management#5, and so on.

We could especially need to define code owners here in every project to clarify who should review what.

In the current context, where we don't have any clear ownership defined (wich is perfectly OK for now IMHO):

• « who asks for review »: the PR author should always ask for a review as soon as the PR is ready
  Reminder: A PR can have multiple reviewers If the first reviewer feels somebody else should also review the PR, they can just send them a review request. And a PR author can ask several persons to review it if they feel its about differents topics involving different ownerships.
• (leading to, I guess) "who should be the reviewer": any blindnet staff members who seem to have a better understanding of the PR context. Right now, it can be simply liked to any manager role:
  • @milstan for everything related to our company's "vision" and high-level concepts (i.e. high-level product ownership)
  • @filipblnt for everything related to engineering, by default (adding a member of the engineering team as an additional reviewer whenever needed)
  • @Vuk-BN for everything related to global executive strategies and finances
  • @noelmace for everything related to communication, openness framework, DevX and DevRel in general
• « favour explicit review requests rather than "everybody should" »: I'm not sure what you mean by that. In my mind, "explicit review requests" is covered by Github PR review feature
• « make sure there is a clearly designated person who should decide (a positive definition - not a definition of who should not) »:
  • the lead reviewer is responsible to decide when to merge a PR
  • cf. the code owner feature, but we can also define a "by default" owner per repository to start with if we need by default lead reviewers
  • My 2 cents: defining who "should decide" goes against our decision framework, which is a process for collective decision making. Once a consensus has been reached (meaning, no one involved in the decision-making process has anything to add, leading to "LGTM"/accepted review comments), anyone can merge the PR (even the author I guess). Of course, if a consensus can't be reached, it is up to the lead review and sponsor to make a decision (which should only concern a part of the global decision, not all of it). Or, if the disagreements are too important, it could simply mean we are not ready to make this decision, and should therefore close the PR and write an alternative decision draft. Again, here, it's pretty much the same as in any open-standard making process. And, again, managers can also simply make a "choice" by themselves (meaning, no decision record).

[milstan]

I agree Noël. My point is:

« favour explicit review requests rather than "everybody should" »: I'm not sure what you mean by that.

Means that the author MUST ask for reviews and reviewers MAY ask for additional reviews. But the decision-making process is not waiting for any reviews from anyone who is not designated as a reviewer. IMHO people who are not marked as being reviewers, nobody is going to wait for their review and the decision-making will go on without them.

This is very different from the current statement about every staff member putting at least LGTM (which we do not require, if I understand well).

[milstan]

Also with regards to collective decision making - yes, in the sense that reviews MUST converge, but at some point if they converge, or not, there is someone (and that IMHO MUST be a well-designated, unambiguous someone) who clicks on MERGE/CLOSE with regards to the presence or absence of concensus. Otherwise the deicisions can be on a stalepoint forever.

[milstan]

So here is what I propose for

decision records, which follow the decision framework, which applies to all projects but only for specific document types as defined in DecisionFramework > what-template-will-you-use

In the beginning there are the authors, one or more community member, one of whom, the Author writes the decision record and is committed to championing it through the process.

If and only if the Author does not have a @blindnet.io e-mail adress, the Author MUST find a Sponsor, a single member of the bindnet organization with a @blindnet.io email address who shepherds the decision record through the review process and makes sure:

• (during the decision making process / PR review) the author is actually heard, the proposal considered, and the process respected
• (after the decision has been recorded / PR merge) the decision is applied, respected and understood (i.e. if someone has a question about this decision, they should reach to the sponsor)

Sponsor, and author(s) SHOULD be clearly defined in a markdown header following the decision framework templates.

Anyone can, and at least one of the {Author, Sponsor} MUST designate one or more reviewers among anyone other then {Author, Sponsor}. A reviewer can desingate themseves.

Author OR Sponsor MUST asign the PR request to one person other then {Author, Sponsor}, capable to merge or close the request, the Asigned. The Asigned looks for concensus before merging a PR. In clear absence of consencsus the Asigned closes the request. It is the responsability of the Asigned to act and not let PR remain at a stale point.

Whenever possible (unless they are one of the {Author, Sumbitter}, or for other reasons known to be unavailable), the Asigned should be:

1. @noelmace for everything related to communication, openness framework, DevX and DevRel in general
2. @filipblnt for everything related to engineering, by default (adding a member of the engineering team as an additional reviewer whenever needed)
3. @milstan for everything related to our company's "vision" and high-level concepts (i.e. high-level product ownership)
4. @Vuk-BN for everything related to global executive strategies and finances

When this is not possible, the then the Asigned SHOULD be the first eligible and avlailable person from the above list, in the order from 1. to 4..

[milstan]

It's not at all clear to people (myself including) how to behave in other situations, so I propose we define also the following for:
other types of documents (e.g. user stories, specifications, designs, documentation work, code) unless a particular project-related workflow is specified in the project's readme file:
The author makes a PR,

• invites reviewers others then themselves
• asignes a person (other than themselves, which may be a reviewer, in the same order as for decision PRs)
• The Asigned looks for concensus before merging a PR. In clear absence of consencsus the Asigned closes the request. It is the responsability of the Asigned to act and not let PR remain at a stale point.

[nweldev]

I'd say we replace "author(s)" and "reviewer(s)" with "lead author" and "lead reviewer" then. All other contributors are just logged in GitHub, but not actually part of the "official" process (meaning the lead author is responsible to deal with other contributors/authors, and the lead reviewer has the responsibility to reach a consensus with other reviewers, on their own terms)

[nweldev]

I'll try to find time to sum it all up in a DR asap and update the documentation accordingly. It will be more efficient to continue this discussion in the associated PR.