Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jgit-dev] Proposal: two-person review submit requirement for JGit

On Tue, Nov 21, 2023 at 3:09 PM Matthias Sohn <matthias.sohn@xxxxxxxxx> wrote:
On Tue, Nov 21, 2023 at 2:38 PM Matthias Sohn <matthias.sohn@xxxxxxxxx> wrote:
On Tue, Nov 21, 2023 at 12:04 PM Jonathan Nieder via jgit-dev <jgit-dev@xxxxxxxxxxx> wrote:
Hi JGit developers,

As part of a postmortem after an outage, some colleagues and I were noticing that more changes in JGit are landing with only self-+2 recently, which leads to fewer people understanding how the relevant code works. I can understand the motivation for that - it's hard to get a reviewer and sometimes a change is obviously good. But it also has downsides in terms of that kind of broader understanding of the code, helping new reviewers ramp up, and reliability and security (insider risk) consequences.

I might be guilty ;-) If it was me, let me know which change caused trouble.
I am doing a lot of reviews, and definitely would appreciate help here.

I am in favor of your proposal.

Though for creation of milestones and releases I'd reserve the right to +2 the version bumping changes
on the respective stable branch and subsequent merges myself if nobody's around.
These changes have to land on the schedule of the Eclipse Simultaneous Release, see [1].
They are mostly created by some scripts [2].
So I'd like to propose the following changes:

 1. Require that all changes have at least a Code-Review+1 from a recognized person other than the uploader. I don't have strong opinions about what "recognized person" would mean here - from a security perspective, it's nice if it comes from some list that prevents a compromised committer from creating a sockpuppet account to do that, but the rest of the motivations would already be satisfied by "any separate account" (since a committer is always involved in a change and can notice if they seem to be the same person).

We could add a submit requirement to validate this and maintain a group with trusted reviewers.
 2. Allow uploaders to hit the submit button to merge a change if it's been sufficiently reviewed. This would reduce friction and compensate for the productivity loss from (1).

I usually submit changes I voted +2 on immediately. If you think it helps we can try this. 
 3. Encourage more reviewers to step up, to reduce the friction of (1). Provide some way to ask for more reviewer attention on a change - is this mailing list the best place for that, or do we want to use e.g. some IRC channel as well?

I can request a public "JGit Reviews" room on which is a matrix deployment run by the Eclipse Foundation.

I requested a chat room in [3] 

The chat room was created [4], please join it for discussing reviews.

Back to the top