Re: [poll] Use Commit Then Review (CTR)

> On Oct 28, 2025, at 12:24 AM, Christian Grobmeier <[email protected]> wrote:
> 
> On Mon, Oct 27, 2025, at 12:25, Gary Gregory wrote:
>> This is a poll to gauge the waters for CTR.
>> 
>> There is way too much Byzantine bureaucracy in this project.
>> 
>> It's constantly throwing spans in the wheels of progress:
>> 
>> A PR for adding one getter method turns into a request for porting a
>> code base from IO to NIO, see
>> https://lists.apache.org/thread/l23k5n6spfgs05ds06t4hpgmmssqpzvd
> 
> -1
> 
> I understand your pain, Gary. We need to make it easier to keep development going.
> 
> But I no longer believe in CTR.
> 
> We had the worst security issue you can imagine. Everything we can do to build up trust is necessary and welcome. We are a critical open-source project in the Java landscape, and we must acknowledge this by ensuring all changes are reviewed.

The security vulnerability was from an external contribution that underwent review before being merged. Given the lack of familiarity of anyone with the exploitability of JNDI at the time (if it was even public knowledge yet), no amount of PR review would have prevented the issue. If that’s the reason why we insist on RTC, then I’d suggest that the review include something that would have prevented the issue rather than simply being a matter of appearances.

> With CTR we have no guarantees anyone ever looks at the change. It is almost certain that a commit is not reviewed at all.

We have no guarantee that any of the existing lines of code have been reviewed. This applies to both our code and our core dependencies (such as OpenJDK, Maven, etc.).

> We are no longer prototyping. Log4j is used around the globe. We changed the whole world's perspective on security.
> 
> Now, Log4j plays a very special role: we lead as a role model in security and development practices. People look at us.

I agree that we should lead as role models. However, that shouldn’t mean adopting bad practices that wouldn’t have prevented security issues. Recall that the most effective tool for finding these sorts of issues (CodeQL) had _explicitly_ marked Log4j as a project to ignore when scanning code. No code reviewer is likely to ever catch a problem involving data flow analysis without using such a tool. Thankfully, we already include CodeQL scanning of PRs (and main branches), so we are proactive about that.

> Strong -1 to going back to CTR.
> +1 on discussing the rules of a review.

The rules of review are the crux of the issue. If review is required, then we must define what standards are required. It’s not enough to simply require code review; nobody agrees what it even means!

> 
> Cheers
> Christian