RFR: JDK-8298445: Add LeakSanitizer support in HotSpot [v6]

[Thomas Stuefe]

On Wed, 8 Feb 2023 03:11:32 GMT, Justin King <jcking at 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