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 >> > > > > > >> > > > > >> > > > >> > > >> > >> >
