You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@polaris.apache.org by Dmitri Bourlatchkov <di...@apache.org> on 2026/07/10 01:27:46 UTC
[DISCUSS] Respecting developer and reviewer cognitive work
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.
Re: [DISCUSS] Respecting developer and reviewer cognitive work
Posted by Robert Stupp <sn...@snazy.de>.
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 <di...@apache.org>
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 <er...@gmail.com>
> 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 <di...@apache.org>
> > 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 <eric.w.maynard@gmail.com
> >
> > > 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 <fl...@gmail.com>
> 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 <
> > > > > dev@polaris.apache.org>
> > > > > 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é <
> > jb@nanthrax.net
> > > >
> > > > > > 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 <
> > > > dimas@apache.org>
> > > > > > > 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.
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] Respecting developer and reviewer cognitive work
Posted by Dmitri Bourlatchkov <di...@apache.org>.
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 <er...@gmail.com>
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 <di...@apache.org>
> 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 <er...@gmail.com>
> > 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 <fl...@gmail.com> 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 <
> > > > dev@polaris.apache.org>
> > > > 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é <
> jb@nanthrax.net
> > >
> > > > > 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 <
> > > dimas@apache.org>
> > > > > > 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.
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] Respecting developer and reviewer cognitive work
Posted by Eric Maynard <er...@gmail.com>.
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 <di...@apache.org>
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 <er...@gmail.com>
> 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 <fl...@gmail.com> 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 <
> > > dev@polaris.apache.org>
> > > 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é <jb@nanthrax.net
> >
> > > > 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 <
> > dimas@apache.org>
> > > > > 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.
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] Respecting developer and reviewer cognitive work
Posted by Dmitri Bourlatchkov <di...@apache.org>.
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 <er...@gmail.com>
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 <fl...@gmail.com> 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 <
> > dev@polaris.apache.org>
> > 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é <jb...@nanthrax.net>
> > > 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 <
> dimas@apache.org>
> > > > 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.
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] Respecting developer and reviewer cognitive work
Posted by Eric Maynard <er...@gmail.com>.
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 <fl...@gmail.com> 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 <
> dev@polaris.apache.org>
> 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é <jb...@nanthrax.net>
> > 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 <di...@apache.org>
> > > 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.
> > > >
> > >
> >
>
Re: [DISCUSS] Respecting developer and reviewer cognitive work
Posted by Yufei Gu <fl...@gmail.com>.
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 <de...@polaris.apache.org>
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é <jb...@nanthrax.net>
> 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 <di...@apache.org>
> > 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.
> > >
> >
>
Re: [DISCUSS] Respecting developer and reviewer cognitive work
Posted by Adnan Hemani via dev <de...@polaris.apache.org>.
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é <jb...@nanthrax.net> 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 <di...@apache.org>
> 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.
> >
>
Re: [DISCUSS] Respecting developer and reviewer cognitive work
Posted by Jean-Baptiste Onofré <jb...@nanthrax.net>.
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 <di...@apache.org>
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.
>
Re: [DISCUSS] Respecting developer and reviewer cognitive work
Posted by Ajantha Bhat <aj...@gmail.com>.
Hi,
I strongly agree that we should not induce new build warnings and ensure
code quality.
PR owners must check it before opening a PR.
Regarding the discussion about deprecation,
There may be downstream projects that are still using these public
interfaces.
So, IMO we still need to mark code as deprecated. Just mentioning it in the
doc may not be enough as not everyone will read the docs always.
Regarding replacing Deprecated usage in code, I agree that we should do it
whenever we deprecate to avoid inducing compile warnings.
We can also follow Iceberg's semantic versioning here for deprecation. So,
the user will know in which version it will be removed by checking javadocs.
https://iceberg.apache.org/contribute/?h=minor#semantic-versioning
- Ajantha
On Fri, Jul 10, 2026 at 6:58 AM Dmitri Bourlatchkov <di...@apache.org>
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.
>