Hi Matt,
Regarding your thoughts on RTC, as you know, I’m personally a strong supporter of the Review-Then-Commit model, especially in the current climate of increasing supply chain attacks and phishing campaigns, it remains one of the most reliable safeguards we have. Please know that this isn’t about trust in you as a person or as an engineer, I have complete confidence in both. My concerns are about two things that go beyond any individual contributor: - Account security: Even well-protected accounts can be temporarily compromised, through tokens, SSH keys, or access from another machine. I know your setup is exceptionally secure (physical second factors for both ASF and GitHub, SSH and GPG keys, etc.), but that’s the exception rather than the rule. For context, even my own authentication tokens are stored in 1Password, and I have copies of them on internet-connected machines. - Human oversight: No matter how careful we are, everyone occasionally overlooks small details. That’s exactly what peer review helps catch. > RTC tends to be used in highly active codebases with dozens or > hundreds of regular contributors. That’s true, but it’s not exclusive to those projects. For example, the `packageurl-java` repository [1] has only one active maintainer, and yet RTC is still used successfully there. In larger projects, RTC (and often a merge queue) becomes essential to keep `main` stable, but it can be just as valuable in smaller teams. > RTC works best when reviewers have sufficient time available every > week. I don’t think time availability necessarily dictates whether RTC or CTR works better. The key question is simply whether every change receives a review. For small or “trivial” commits, the review itself is usually trivial, so I’d still prefer having that trace of peer validation. Personally, I’m flexible about the *order* of review (before or after commit), as long as every commit is reviewed by someone other than the author. If we ever decided to adopt a true CTR model, where every commit is traceably reviewed, I’d be fine with that too. > RTC can technically be accomplished by having a coworker review the > code before making a PR... That’s a fair point, and it highlights one of the challenges of commercial open source: employer pressure can sometimes lead to rushed or superficial reviews. Fortunately, that’s not really an issue for us: none of us are full-time contributors here. In fact, it’s quite the opposite: if your employer allows you to dedicate even 10% part of your time to Log4j, that’s a generous contribution. (For reference, the small Tidelift sponsorship Volkan and I receive is far below that.) > RTC seems to think that the ideal time to go over architectural design > and other planning details is at the time of proposing the code > change... I completely agree with this concern, but I don’t think it’s a limitation of RTC itself: it’s more about how we apply it. When a PR introduces an architectural change, we should indeed move that discussion to the mailing list or GitHub discussions (which are mirrored there) and hold the PR until consensus is reached. > RTC is a flow optimized for accepting contributions from third > parties... Outside of very active projects, I think the timing of review (before or after merging) matters less than ensuring the review happens at all. That’s where the real quality and trust come from. In summary, I don’t think our discussion is really about “RTC vs CTR”, but about ensuring that every commit to `main` (or `2.x`) is reviewed by someone other than the author. I believe that’s essential for maintaining quality and shared ownership. Personally, I prefer to have reviews happen before merging, but if you prefer to merge first and review afterward, that’s also fine: as long as every commit is eventually reviewed before we cut a release. Both approaches have trade-offs: - With RTC, pull requests can sometimes linger unreviewed for too long. - With CTR, there’s a risk that older commits slip through without ever being reviewed. We’ve seen how that can happen in practice. For example, last year Commons Compress gained two new dependencies that went unnoticed by anyone except the original committer, and even the changelog didn’t mention them until months later [2]. That was a security release, which makes the situation even more delicate. When urgency is high, it becomes harder to balance the need for timely fixes with the discipline of thorough review [3]. So, whichever workflow we use, the key is to make sure that review actually happens and that we help each other keep that process moving. Best, Piotr References: [1] https://github.com/package-url/packageurl-java [2] https://github.com/apache/commons-compress/commit/804c4fd9bc7dba9826c6b938cff2d6fdfccc5dc4 [3] https://lists.apache.org/thread/5vtbbs18d4rntn8tpnpsvo0g9h6svc30
