Respect between contributors and reviewers should be mutual. I don't think introducing a warning for a well justified reason should block a PR. If the community believes that no new build warnings should ever be introduced, even for intentional deprecations, then I think we should make that an explicit rule rather than an arbitrary behavior that reviewers apply on a case-by-case basis.
Having a clear, documented rule gives contributors predictable expectations, avoids double standards, and ensures review decisions rely on community agreed guidelines rather than individual reviewer preferences. That ultimately leads to a fairer and more consistent review process for everyone. Yufei On Thu, Jul 9, 2026 at 10:03 PM Adnan Hemani via dev <[email protected]> wrote: > Hi JB, > > Can you explain what you mean by "fully acceptable if explained"? In the > case that came up (in the PR linked in the original message), > the @Deprecated tag was being used to alert end users who may be using a > particular config. In your opinion, is that a reasonable cause for being > "fully acceptable"? > > Best, > Adnan Hemani > > On Thu, Jul 9, 2026 at 9:56 PM Jean-Baptiste Onofré <[email protected]> > wrote: > > > Hi Dmitri > > > > I fully agree that we should avoid introducing build warnings. If we do, > we > > must clearly document the reasons (especially for the reviewer). > > > > It's an implicit good practice, in my humble opinion. Also, I would > > consider it's up to the reviewer to remind the contributor of that good > > practice. > > It's certainly not a hard rule, but a good practice, and it's fully > > acceptable if explained. > > > > Regards > > JB > > > > On Fri, Jul 10, 2026 at 3:28 AM Dmitri Bourlatchkov <[email protected]> > > wrote: > > > > > Hi All, > > > > > > This is to follow-up on review comments in PR [5012], specifically [1] > > > > > > People working with the codebase on a regular basic have to pay > attention > > > to many factors to ensure code quality. This requires a significant > > > cognitive effort. > > > > > > One of the factors is the presence of build warnings. I believe it is a > > > generally good practice to avoid introducing new build warnings when > > > technically possible. > > > > > > Thoughts? > > > > > > [1] https://github.com/apache/polaris/pull/5012#discussion_r3547988353 > > > > > > [5012] https://github.com/apache/polaris/pull/5012 > > > > > > Thanks, > > > Dmitri. > > > > > >
