Hi guys, Regarding the already "resolved" code-owners: I agree that it should not be added, generally agreeing to the previously raised concerns.
Adding a technical enforcement for two reviewers is too strict, because it also means that literally all changes, even the smallest typo fixes and all the patch version bumps would require two committers to approve. It would also not count approvals from community members that are not (yet) committers. Robert On Mon, Jan 5, 2026 at 10:48 AM Jean-Baptiste Onofré <[email protected]> wrote: > > Here's the PR about required approval from 2 reviewers: > https://github.com/apache/polaris/pull/3353 > > Regards > JB > > On Mon, Jan 5, 2026 at 10:41 AM Jean-Baptiste Onofré <[email protected]> > wrote: > > > Hi folks, > > > > Thanks for your feedback. > > > > I will close the current PR and create a new one proposing a requirement > > of two reviewers, as I suggested a few weeks ago. > > > > Regards, > > JB > > > > On Fri, Jan 2, 2026 at 9:19 PM Prashant Singh via dev < > > [email protected]> wrote: > > > >> Hi JB, > >> > >> Thank you for the proposal. > >> > >> I’m open to trialing this, though I agree with others that this change may > >> not significantly impact my personal workflow. Before we move forward, I’d > >> like to clarify a few details regarding the logistics: > >> > >> - > >> > >> *Selection Criteria:* How were the designated reviewers for each module > >> selected? Was it based on commit history, past review activity, or > >> another > >> metric? > >> - > >> > >> *External Feedback:* How will we handle concerns raised by > >> non-designated reviewers? I firmly believe we should prioritize > >> "community > >> over code"—if a community member raises a valid point, we should reach > >> a > >> consensus rather than bypassing it based on reviewer status. > >> > >> Regarding the speed of merges: I believe "merging too fast" is an > >> oversimplification of the core issue. The real concern is building > >> consensus on implementation and goals, and providing the community > >> sufficient time to provide feedback. While typos or urgent bug fixes can > >> be > >> fast-tracked, the same cannot be said for new features—especially those > >> that add dependencies on external projects, reinvent existing logic, or > >> complicate the design *without explaining what alternatives were > >> considered > >> and building consensus around them.* > >> > >> I really like the suggestion made in the other thread [1] regarding a > >> requirement for two reviewers; I believe this would be a great step toward > >> ensuring quality and consensus. > >> > >> [1] https://lists.apache.org/thread/hzxds729v5r68togbfx76l14f9m4bfj4 > >> > >> Best, > >> > >> Prashant > >> > >> > >> On Wed, Dec 31, 2025 at 10:36 PM Jean-Baptiste Onofré <[email protected]> > >> wrote: > >> > >> > Hi folks, > >> > > >> > The proposal addresses the issue of PRs being merged too quickly by > >> > requiring approval from a designated reviewer before a merge can occur. > >> By > >> > combining module-specific reviewers with the requirement for at least > >> one > >> > formal review, we can ensure better oversight. > >> > > >> > Ultimately, my goal is simply to improve the current process. > >> Personally, I > >> > don't see an issue with PRs being merged quickly as long as the relevant > >> > reviewers are satisfied with the changes. > >> > > >> > For context, we discussed that during the community sprint last month, > >> so > >> > it's just a proposal. If we are fine with the current process, that's > >> OK, > >> > and totally fine to merge a PR super fast. > >> > > >> > Regards, > >> > JB > >> > > >> > On Tue, Dec 30, 2025 at 1:03 AM Yufei Gu <[email protected]> wrote: > >> > > >> > > Thanks JB for the proposal of reviewer notification! > >> > > > >> > > I generally agree with Dmitri here. For me personally, this probably > >> does > >> > > not make much difference either, since I am already getting pinged on > >> PRs > >> > > anyways. > >> > > > >> > > My main concern is that having a list of module reviewers does not > >> really > >> > > address the core issue raised, namely PRs being merged too quickly > >> > without > >> > > sufficient oversight. As far as I know, this has never really been a > >> > > problem of the right people not being notified in the first place. > >> > > > >> > > I am neutral to this effort overall, and happy to see it tried if the > >> > > community thinks it helps. I am just a bit hesitant about adding more > >> > > process and complexity without a clearer benefit or a stronger > >> guarantee > >> > > that it actually improves review quality. I'm pretty sure people may > >> have > >> > > different thoughts on whether he/she should be on which lists. One > >> way to > >> > > move forward is to empty all lists as the initial PR. People can add > >> > > themselves voluntarily if they want to get notified, however, it may > >> not > >> > be > >> > > worth the effort overall though. > >> > > > >> > > Thanks, > >> > > Yufei > >> > > > >> > > On Mon, Dec 29, 2025 at 6:32 AM Adam Christian < > >> > > [email protected]> wrote: > >> > > > >> > > > Howdy JB, > >> > > > > >> > > > I like this idea. > >> > > > > >> > > > I believe that this is a natural step given how the codebase and > >> active > >> > > > participation has grown. This helps us solve a few problems: > >> > > > 1. For subject matter experts, it allows them to know which reviews > >> > truly > >> > > > need their attention without having to sort through an inbox. > >> > > > 2. For new contributors, it allows them to know who understands the > >> > > > codebase, so they can get help as they are onboarding. > >> > > > > >> > > > Go community, > >> > > > > >> > > > Adam > >> > > > > >> > > > On Wed, Dec 24, 2025 at 11:04 AM Dmitri Bourlatchkov < > >> > > > [email protected]> wrote: > >> > > > > >> > > > > Hi JB, > >> > > > > > >> > > > > Using the auto-labeller to notify some specific people on PRs > >> sounds > >> > > > > reasonable to me. > >> > > > > > >> > > > > For me personally, it probably makes little difference as I get > >> > > > > notifications for all PRs anyway :) However, I'm willing to > >> > participate > >> > > > and > >> > > > > see how the new system works in practice. > >> > > > > > >> > > > > Cheers, > >> > > > > Dmitri. > >> > > > > > >> > > > > On Wed, Dec 24, 2025 at 2:08 AM Jean-Baptiste Onofré < > >> > [email protected]> > >> > > > > wrote: > >> > > > > > >> > > > > > Hi folks, > >> > > > > > > >> > > > > > Some time ago, we decided to remove auto-reviewers on PRs to > >> > prevent > >> > > > > email > >> > > > > > flooding and increase the "velocity". However, we have recently > >> > > > discussed > >> > > > > > concerns regarding PRs being merged too quickly without > >> sufficient > >> > > > > > oversight. Additionally, several contributors have volunteered > >> to > >> > > help > >> > > > > > review changes for specific modules. > >> > > > > > > >> > > > > > To address this, I have drafted a proposal for module-specific > >> > > > reviewers: > >> > > > > > - PRs will be automatically labeled based on the modules > >> affected. > >> > > > > > - A specific set of reviewers/experts will be automatically > >> added > >> > to > >> > > > the > >> > > > > PR > >> > > > > > based on those labels. > >> > > > > > > >> > > > > > The goal is to ensure the right "experts" are notified without > >> > > > > overwhelming > >> > > > > > everyone with notifications. > >> > > > > > > >> > > > > > I have created a PR to illustrate how this would work: > >> > > > > > https://github.com/apache/polaris/pull/3328 > >> > > > > > Please note that this is an initial draft of the labels and > >> > > reviewers, > >> > > > > and > >> > > > > > we can refine the lists as needed. > >> > > > > > > >> > > > > > Thoughts? > >> > > > > > > >> > > > > > Regards, > >> > > > > > JB > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > >
