I agree with Russell 100%. I think the intent here is to codify a spirit of
collaboration. That’s always going to be imperfect. But clarifying
expectations is important, even if it’s imperfect.
For some of the requirements (e.g., waiting an appropriate period or
waiting merging with unresolved comments), I think there are ways to set up
merge gates that can be bypassed with a label. I know we have something
similar setup at Snowflake. E.g., if someone can’t wait two days, they
could add a label that allows bypassing the merge gates. I can look into
how that works, if you like.
Regarding the "stable APIs", personally I think we should bias toward open
rather than bias toward closed. If an interface is marked public and in a
package that's not specifically marked for internal use, it should be
assumed to be open. Biasing closed means that we have to explicitly
annotate APIs that are intended to be publicly accessible - people browsing
the code may not know that that's the convention we've gone with. I know I
certainly don't read the contributing guide for every library I add a
compile-time dependency on. Biasing open means we have to explicitly
annotate APIs that are expected to be internal only. Those annotations and
the javadoc comments make it clear for any user who's trying to add a
compile-time dependency on something, making it glaringly obvious that they
should not do so (or at least, that doing so they are explicitly taking the
risk that something will change without warning). I think the bias should
also follow similarly for stable v experimental APIs. Anything not
explicitly annotated as @Experimental is considered stable.
More than java interfaces, we have to be cognizant of everything
user-facing - this includes the configuration files, health checker
endpoints, metric names... there are people who currently have Polaris
deployed to production and some of them have alarms and dashboards that
matter to them.
I acknowledge that this is a tough balancing act. Iteration speed matters
and it's important that we have the ability to make mistakes and go back
and fix them - otherwise we end up in analysis paralysis because nobody
wants to merge the imperfect API that has to last forever. It should be ok
to merge mistakes and to go back and fix them, but I think we can do that
and still have good general guidelines that move everybody toward more
communication and more collaboration, even if those guidelines aren't
perfect.
Mike
On Wed, Jan 22, 2025 at 11:15 AM Russell Spitzer <russell.spit...@gmail.com>
wrote:
> The proposal sounds good to me, I really don't think the details matter
> that much here.
>
> If someone is arguing the wording of the guidelines ("I waited two
> mercurial days, it didn't specify in the guidelines.")
> then something has gone wrong just as much as having broken them. Even if
> we specified the
> guidelines so well that no one could argue against their wording, we
> wouldn't use the
> guidelines as the justification for an action in the repository. If you
> attempt to follow the guidelines
> and make a mistake, or even follow them accurately but find out that by
> no-ones fault someone
> who should have been notified wasn't able to be you should work to find a
> common
> ground that doesn't just involve "too bad, I followed the rules."
>
> There will be exceptions to these guidelines as well I'm sure, we just need
> to be clear that if you are
> violating these principles you have a very good reason for doing so.
>
> For example for an immediate security purpose, just make sure you can
> clearly articulate why that
> merge needed to happen immediately and be prepared to defend it to the
> community.
>
> Need to open a duplicate of a pr that was already closed? I'm sure there
> are situations in which you
> might want to do so. You can do so, but you better have a reason that isn't
> "I wanted a different outcome".
> Did circumstances change? Has the project evolved in a different direction?
> Someone left the project?
> All of these could be valid reasons and I don't think we gain anything by
> articulating everything.
>
> The key principle here is to treat other contributors as collaborators and
> I fully approve of that. I don't feel
> strongly about encoding the rules in github itself.
>
>
> On Wed, Jan 22, 2025 at 12:05 PM Dmitri Bourlatchkov <di...@apache.org>
> wrote:
>
> > On Wed, Jan 22, 2025 at 12:58 PM Jean-Baptiste Onofré <j...@nanthrax.net>
> > wrote:
> >
> > > It's not a problem to have several people working on the same feature.
> > >
> > > By duplicate PRs, I mean that you open a PR, where you get comments,
> > > changes requested, etc. Then, you close this PR to open a new one (on
> > > the same topic). So, I mean by a single person. I'm more in favor of
> > > refactoring an existing PR, instead of closing PR #1 to open PR #2
> > > with the refactoring.
> > > On some projects, I saw contributors closing the PR #1, creating PR #2
> > > just to get approval and merge (loosing the discussion on #1).
> > >
> >
> > Thanks for the clarification! I totally agree with this.
> >
> > Cheers,
> > Dmitri.
> >
>
