Re: [BuildStream] Proposal: spreading responsibility and maintainership [Was: 1.4 Release timeline]

[Tristan Van Berkom <tristan vanberkom codethink co uk>]

On Wed, 2018-12-12 at 09:20 +0000, Laurence Urhegyi wrote:
> On 2018-12-12 06:20, Tristan Van Berkom wrote:
[...]
> > While people can discuss their branches outside of gitlab, we cannot
> > produce a clean feedback loop which excludes WIP stuff with the current
> > gitlab limitations, and I strongly believe that including developers in
> > the feedback loop is a necessary step towards empowering developers and
> > putting the developers in the driver seat.
>
> I wonder how other projects get around this?
>
> I don't think it is too much to ask to ask those people who are doing 
> full notification subscription to filter emails to avoid WIP MRs.

Honestly, I feel that the whole github/gitlab family of software is
targetting small or slow moving projects, web applications mostly.

It is probably just not a high priority with a majority user base that
has a lot of small separate projects - it's possible that the
enterprise users structure their work in a lot of mini repositories and
have teams working on small separate components in separate
repositories (which again can possibly make a lot of sense in the cloud
computing arena).

That said, from Angelos's reply we might have a technical solution to
work around this problem, which would be great.

[...]
> > I am more and more ambivalent about the approvers; especially if there
> > is only one required approver and that those who have sufficient
> > permissions can self approve.
>
> I really like it. Again, comments from others are welcome.
>
> One thing to consider is whether we try to categorise the codebase (as I 
> started on the wiki and we could do with CODEOWNERS) and assign relevant 
> approvers per area, or just give blanket access. I think the codebase is 
> probably large enough to warrant the former.
>
> > My main worry with approvers comes from my experience with gerrit's
> > voting system, which I believe allows people to "pass the buck" when it
> > comes to reviews, at least in the settings where I've used gerrit.
>
> I don't know about gerrit's voting system but it sounds like a different 
> thing to me.

While my concern about this is less than before, and obviously a
majority of people seem to like the approvers so I don't want to stand
in the way... I will reiterate what my original concerns were for
clarity.

Gerrit had a sort of "+1" system where everyone could give their "+1",
and some people had power to give more than one, like a "+2",
interestingly... with gerrit, 1 + 1 does not equal 2, you need an
explicit +2.

The way we had set it up, you needed a +2 for a merge (so a "+1" is
like an approval, and a "+2" is a merge).

What I noticed happening is a workflow like this:
* Someone writes a patch
* A hand full of people look at the patch, and feel confident enough to
  give it a "+1" (or an approval), but not confident enough to dare to
  give it a "+2" (merge).
* The patch sits there for a while
* On a later date, someone on IRC says "Hey I really need this
  functionality, and there are a lot of approvals, can we please
  merge this ?"
* At this point, it's possible we're frustrated enough to go ahead
  and merge the patch based on a hand full of "+1"s, but in the
  absence of a concrete "+2" (we can at times end up giving it a +2
  without anyone having really been accountable for the review).

Essentially my concern is about accountability; it's very easy for
someone to throw out there that they "Approve" something if they are
not on the line for merging it, and multiple "Approvals" don't
necessarily mean someone has reviewed it thoroughly and really is
confident enough to hit "Merge".

The way you seem to have it setup in BuildGrid, where only one approval
is required, the approval would *be* the review, and the merge is not a
consequence of a mob, but of a single reviewer who assumes full
accountability for the review, which is better I think.

Cheers,
    -Tristan