Hi, I think there are two separate questions here.
First, the narrow PR #5012 question: What should happen if a Java symbol is annotated with `@Deprecated` while Polaris itself still uses it? I think that Java deprecation warnings are library/API-level signals to code contributors. If Polaris emits those warnings in its own normal build, those warnings should be actionable for Polaris developers. For known compatibility paths, we should either migrate internal usages to the replacement in the same PR, or suppress the remaining compatibility usages locally with a short comment. Otherwise every build teaches contributors to ignore deprecation warnings, which makes the signal less useful. Second, there is the broader question of how to track feature/config deprecations. For feature flags, users usually do not see Java compiler warnings. So I do not think `@Deprecated` should be the primary tracking mechanism for a functional feature-flag deprecation. That should be captured in docs, changelog/release notes, a GitHub issue or removal plan, and runtime/readiness warnings when the deprecated config is actually used. This is related to the earlier docs discussion about not confusing deprecated compatibility/access paths with deprecated configuration features [1]. I do not think we need a new blanket rule like "never introduce warnings". The narrower expectation is: Use the warning channel for the audience that can act on it, and keep normal Polaris builds free of known non-actionable warnings where practical. I do not think Dmitri's PR comment depends on establishing a new project-wide rule first. For this PR, the concrete review ask is narrower: Avoid introducing known, non-actionable internal deprecation warnings when the migration and compatibility boundaries are already identifiable. For PR #5012, that means migrating tests to `ALLOW_EXTERNAL_METADATA_FILE_LOCATION` where that is the intended replacement, and suppressing/commenting only the remaining backward-compatibility usages of `ALLOW_EXTERNAL_TABLE_LOCATION`. Robert [1] https://lists.apache.org/thread/5rf8q2k8g54f9x78h1lwyvr3yb3pllnd On Sat, Jul 11, 2026 at 1:23 AM Dmitri Bourlatchkov <[email protected]> wrote: > Hi Eric, > > This discussion is starting to blend with the comment trail in PR 5012 a > bit (link in my first message). > > Feature flag deprecation is interesting, indeed. A "production readiness" > checks can be effective in warning end users about altertering the > deprecated feature flag's value via configuration settings (-D, files, > env., etc.) > > The point I'm trying to emphasise in this thread is not so much about > specific changes we need for PR 5012, but a general approach of applying > the migration strategy for the deprecated flag at internal usage sites. > > For example, if existing tests use ALLOW_EXTERNAL_TABLE_LOCATION to allow > metadata files to be placed at arbitrary locations, they could probably be > updated to use ALLOW_EXTERNAL_METADATA_FILE_LOCATION instead. At least this > appears to be the suggestion in the original deprecation email [1] > > Doing that removes the irritating build warnings and also informs > downstream users about a possible migration path. > > On top of that we can suppress the warnings at usage sites that provide > backward compatibility (e.g. in the production readiness checks). > > All in all, the end result is going to be nice and meaningful both for > internal project developers and downstream users, I think. > > [1] https://lists.apache.org/thread/t2pn7t2h983qjpkzq7n08cxxv941n8pm > > Cheers, > Dmitri. > > > On Fri, Jul 10, 2026 at 6:59 PM Eric Maynard <[email protected]> > wrote: > > > That’s a good distinction to make, though I think feature flags sit in a > > somewhat unique position here. They are expressly intended for users to > > configure runtime behavior without writing a line of code (and to keep us > > honest about regressions & deprecations). > > > > We really want to discourage anyone — outside or inside the project — > from > > relying on flags we plan to deprecate and build warnings are one of the > few > > levers we have to do that. > > > > Instead of marking the flag itself as deprecated, I suppose what we > really > > want to deprecate is setting the flag to a nonstandard value (in this > case > > True). We could do that at runtime — the first time we read a nonstandard > > value from a flag marked as deprecated, we could log. Does that work > > better? > > > > —EM > > > > On Fri, Jul 10, 2026 at 3:20 PM Dmitri Bourlatchkov <[email protected]> > > wrote: > > > > > Hi Eric, > > > > > > In my view, deprecating a method is a signal from a library (such as > > > Polaris in this case) to its downstream users. In this context, it is > > > indeed intended to produce build warnings to entice downstream users to > > > update their code and use a supported alternative. > > > > > > Within a project itself there is little value in deprecating a method > > > internally to cause build warnings. This creates nothing but > distraction > > > for people working with the project's source code. I believe all > internal > > > usages of the deprecated method should switch to the supported > > alternative > > > at the same time as the deprecation annotation is added. > > > > > > The only common exception is supporting existing _external_ users of > the > > > deprecated method / variable to allow them to migrate gradually. > > > > > > The gradual aspect, however, is not relevant to internal usages since > at > > > the time of deprecation a supported alternative must already exist, > > > otherwise the deprecation is not meaningful. > > > > > > Cheers, > > > Dmitri. > > > > > > On Fri, Jul 10, 2026 at 6:07 PM Eric Maynard <[email protected] > > > > > wrote: > > > > > > > If the community resolves to never introduce build warnings, what > > exactly > > > > would the utility of the @Deprecated annotation be? My understanding > is > > > > that it primarily exists to intentionally add a build warning, > > prompting > > > > downstream projects to not rely on some functionality that will later > > be > > > > removed. Is the resolution you’re actually advocating for just “don’t > > use > > > > @Deprecated”? > > > > > > > > —EM > > > > > > > > On Fri, Jul 10, 2026 at 3:00 PM Yufei Gu <[email protected]> > wrote: > > > > > > > > > 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. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
