Hi folks I'm updating the corresponding PRs with our discussion during the last Community meeting.
I will keep you posted. Thanks ! Regards JB On Thu, Jan 23, 2025 at 11:14 AM Jean-Baptiste Onofré <j...@nanthrax.net> wrote: > > Hi, > > Let's rename from "guidelines" to "good practices & advices" :) > > The purpose here is not to have strict enforcement, it's more to > assist contributors about good practices to have a better > collaboration. > There's no problem to "deviate" from these good practices with good > reasons and transparency. > > The idea is more to "frame" what we discuss. > > In this regard: > - I propose to create a PR to update the contributing guide with this > good practices discussed in this thread > - I propose to close https://github.com/apache/polaris/pull/840 in > favor of "soft rule" (we quickly discuss about owner review with > CODEOWNERS in this PR, but let's keep aside for now) > - I propose to update https://github.com/apache/polaris/pull/839. We > have a consensus about the label, but no consensus about the schedule > for now. In order to move forward, I propose to update the PR with > just the label for now (as we have a consensus) and see if it's > enough. If it's still a concern, I will create a new PR to update the > schedule > > Thoughts ? > > Thanks ! > Regards > JB > > On Thu, Jan 23, 2025 at 5:39 AM Michael Collado <collado.m...@gmail.com> > wrote: > > > > 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. > > > > > > >