Document where admin-bypass of branch protection can be used.

[k-wall]

Relates to: https://kroxylicious.slack.com/archives/C04V1K6EAKZ/p1744749122953979

[tombentley]

Shame on us that it's been sitting here for nearly a year.

@k-wall I think if you still want to merge it we need to tweak the language "Code Owner" -> "Committer".

[k-wall]

I think there is still value in writing it down. I've updated the words. It needs a PM rather than Committer, as it is PM who have admin rights. PHAL @tombentley

[tombentley]

I have no problem with the spirit of this.
However, I think in practice there are few factors in play which we should consider:

• I find some of the Sonar warnings to be of dubious value. To the extent that it sometimes feels like cargo cult programming to me. By gating on low-value checks we're actually just encouraging people to perform the rituals, in order to get their change merged, rather than using their own judgement. In contrast I'm so far (it's still early) finding that the ErrorProne checks are less likely to be low value pedantry.
• I'm actually quite suspicious about where SonarCloud's whole "code quality grading" comes from. I've only ever see it grade our code at A or C, never B. That seems odd. Is this "grade" actually backed up by any proof that it correlates with objectively better software?
• I think that between Checkstyle and ErrorProne we've actually got a lot of the same coverage that Sonar gives us. Maybe they're not a complete replacement, but they're petty good.
• I've not yet added support for compiling with the checker framework (is a change I have locally). That seems like a route to having even better static analysis of our code, and actually about the kinds of problem that I actually care about. That is real bugs, rather than vague and sometimes dubious "best practices".
• Checkstyle and ErrorProne are much preferable in at least one respect: they break the mvn build (i.e. fail fast locally). In contrast, checks that cannot be done from the command line are invisible to coding agents. It would be a much better experience, over all, if the agent is generating good code all the way through a big multi-commit change, rather than the contributor only finding about all the rules they're breaking when they push and someone triggers the checks to run. That pushes contributors into a workflow of adding fixes after the fact and then probably some branch surgery to tidy up the history.

Putting all these points together, it just feels a bit weird to be elevating Sonar like this. This rule you're wanting to add is actually only necessary because SonarCloud does not integrate with the local mvn build. You could almost say because SonarCloud is a bad tool.

[k-wall]

Yes, I did wonder if we could rationalise our checker tools. We had a case earlier this week where Sonarcloud pulling the code one way and Errorprone another (defaults in switches). We'll probably see more cases like this.

Is this something you are planning to proposal already?

[tombentley]

It's not a priority at the moment. But the checker framework does have some interesting mini type systems which would be relevant to us and could help detect real issues. Thread safety/Locking and tainting untrusted or sensitive data are two which I'd like to experiment with.