I like this idea. However, I very rarely miss a notification, which may not be the case for others. Being blocked when you have a 1 review and awaiting a 2nd can be frustrating.

I'm 👍 to this idea. Two members or associate members?

Perhaps after we've tagged "v1" of our template and the repo is citable, etc...?

I'm actually against this. This morning (until @matt-graham) arrived, we only had 2 of us. Plus, people often take ages to review, so it will make it hard to get stuff merged.

Notes from hackmorning 2024-10-22

Discussion

There is a bit of a distinction between opinionated changes (i.e. changes to the traffic lights and default tools in the template) vs infrastructure fixes, linting, or fixes to tests.

The CONTRIBUTING guide currently (df14157) says:

The website pages are open to contributions but they will need to be reviewed by a member or associate member of ARC. We might be slow to approve new tool suggestions (since we'll probably want to discuss them first) but don't let that put you off creating an issue.

Our quick zoom around rulesets doesn't seem that we could have a per-directory rule. (2 reviewers for docs/ and only one reviewer for everywhere else.)

There hasn't yet really been any change that was close to controversial that made it to the repo before a long discussion in issues. Maybe this is not such a problem.

Decisions

We will have a common-sense policy: two reviewers if you think it's something potentially controversial like changing a traffic light. One reviewer otherwise. This is not enforced but included in CONTRIBUTING. @UCL-ARC/collaborations-python-tooling can be used for "the second" reviewer.

Happy to stick with this for now - I do think there's a bit of a chicken and egg situtation though. If we only require one reviewer, the more regular reviewers on this repo will review and merge everything, thus not encouraging others to join in and review, thus reducing the reviewer pool so that we are worried that requiring two reviewers means stuff isn't merged.

I personally think the benefit of encouraging new maintainers in this repo outweighs the downside of sometimes having to wait a bit for review, and I don't think this is a repo where we have to merge stuff immediately and develop fast. Indeed, I think it's much more important to have consensus (even on seemingly small infastrcuture changes) than merge stuff fast.

Happy to go with the above though, thanks for discussing and documenting that! 👍

One suggestion I would make is that the first reviewer is allowed to request a second reviewer, and the author should respect that decision?

I've also made an issue label needs-2-reviewers