On Wed, 8 Feb 2023 03:11:32 GMT, Justin King <jck...@openjdk.org> wrote:
> > The bot can only check the required number of Reviewers (strictly 1 per > > OpenJDK rules but changeable as here via `/reviewers` command) but it > > doesn't know about the informal rules such as having reviewers from each > > affected area (there is no mapping from people to areas). > > Regardless until your conversation with @tstuefe was complete this was not > > ready for integration. Yep, that was a bit quick. But misunderstandings happen. > > From my point of view it was more questions to clarify how it worked at this > point to get to an understanding, rather than specific objections, since the > flags were no longer there. Hi Justin, No, the discussion was still ongoing. The informal rule is that ongoing discussions should be closed and that nobody strongly objects to a change. Two reviewers are easy to come by. The point of this rule is that you have a reasonable chance to block changes you strongly oppose. And that every change in the code base has the buy-in from the majority. Note that a patch is seldom blocked by one person alone for a prolonged amount of time. But sometimes, overworked reviewers may block-then-ghost without meaning to. Requires patience from authors and responsibility from reviewers. As an author, it is okay to ping after a while and, getting no answer, to integrate with a clear warning. This all requires more time than faster paced projects, which is accepted for most RFEs. Requires more discussion and convincing, but gives us a better community. Still faster than getting in kernel patches :) > Requiring contributors to adhere to informal rules such as who can and cannot > approve, and who you should and do not have to wait for, is not ideal. It > relies heavily in determining who is part of what group and what group they > are reviewing for. The rule is simply to resolve all discussions and opposition. In fact, even if you are an unknown, come from the outside, and oppose a patch with good reasons, people will usually listen. If the points made were good, the author has to address those concerns (or, should). > Wouldn't it make more sense to tie approval to labels and have reviewers > explicitly approve via a command? Or allow reviewers to explicitly require > their approval via a command? That way there is no guessing? It would at > least make it explicit and avoid miscommunication. The reviewers themselves > are in the best position to know and bump required reviewers, not > contributors. Our rules come from pre-Github times, where discussions happened on MLs. We may adapt over time. But you cannot codify all interactions into software-enforced rules, there is always a human element. (I will continue the technical discussion separately) Cheers, Thomas > > Just my two cents. ------------- PR: https://git.openjdk.org/jdk/pull/12229
