Sounds good to me! On Wed, Mar 26, 2025 at 11:42 PM Jean-Baptiste Onofré <j...@nanthrax.net> wrote:
> Hi Eric > > I agree. I propose: > > 1. To update the contributor good practices on the website to clearly state > the keywords nit and minor. > 2. For request change, we can use it. For the record, we can bypass request > change flag if a given number of reviewers approve the PR anyway. > > Thoughts ? > > Regards > JB > > Le mar. 25 mars 2025 à 19:54, Eric Maynard <eric.w.mayn...@gmail.com> a > écrit : > > > In this case I think rather than a mistake, there could simply be some > > ambiguity around what comments are considered blocking. For example, I'm > > not sure if a comment prefixed "minor" should be considered blocking -- I > > would say probably not, but someone else could interpret it differently. > > > > To that end, I think putting these things in the guidelines could be wise > > even if there's not a strict enforcement of the guidelines as such. I > agree > > that it's hard to "codify a spirit", but it could be useful to have > > explicit expectations and guidelines written down somewhere. It could be > as > > simple as asking reviewers to "Request Changes" if they want to block a > PR, > > or advising authors to wait a couple of days before merging a PR they > view > > as complex. > > > > --EM > > > > On Mon, Mar 24, 2025 at 11:13 AM Yufei Gu <flyrain...@gmail.com> wrote: > > > > > I'm particularly concerned about what happened in PR #1230( > > > https://github.com/apache/polaris/pull/1230). The PR was merged > despite > > > multiple unresolved comments from a committer. > > > > > > To maintain a healthy review process, we’d like to encourage PR authors > > to > > > address open comments and work toward consensus. If necessary, relying > on > > > lazy consensus is fine. > > > > > > Let’s all try to be mindful of this going forward to ensure smooth > > > collaboration. > > > > > > Yufei > > > > > > > > > On Mon, Mar 24, 2025 at 1:55 AM Jean-Baptiste Onofré <j...@nanthrax.net> > > > wrote: > > > > > > > Hi Eric, > > > > > > > > Thanks for pointing this out. > > > > > > > > I think that's unfortunate, especially for #1230 (I don't see issues > > > > on #1226 and #1220 as they have been approved by 3 different > > > > committers). > > > > > > > > I strongly believe that, as a community, we do a great work all > > > > together, and we highly consider comments from each other. > > > > If again, I think it's unfortunate for #1230, it's certainly a > > > > "mistake" from the author/merger and we should still give a change to > > > > our soft rules (before being stricter). > > > > > > > > So, I propose to continue with our soft rules and good intentions, > and > > > > if it doesn't work, then we can discuss about a stricter approach. > > > > > > > > Thoughts ? > > > > > > > > Regards > > > > JB > > > > > > > > On Sun, Mar 23, 2025 at 8:05 AM Eric Maynard < > eric.w.mayn...@gmail.com > > > > > > > wrote: > > > > > > > > > > Revisiting this thread, I wonder if the "soft rules" are working. I > > > have > > > > > noticed quite a few PRs merged recently with outstanding comments. > > The > > > > most > > > > > recent of these that I personally reviewed are #1220 > > > > > <https://github.com/apache/polaris/pull/1220>, #1226 > > > > > <https://github.com/apache/polaris/pull/1226>, and #1230 > > > > > <https://github.com/apache/polaris/pull/1230> but there are > > doubtless > > > > other > > > > > examples. > > > > > > > > > > If, indeed, the guidelines are not working perhaps stricter > > enforcement > > > > > more consistent with JB's initial proposal would be effective. > > > > > > > > > > On Fri, Jan 24, 2025 at 8:21 AM Dmitri Bourlatchkov > > > > > <dmitri.bourlatch...@dremio.com.invalid> wrote: > > > > > > > > > > > On Thu, Jan 23, 2025 at 5:15 AM Jean-Baptiste Onofré < > > > j...@nanthrax.net> > > > > > > wrote: > > > > > > > > > > > > > Let's rename from "guidelines" to "good practices & advices" :) > > > > > > > > > > > > > > > > > > > +1 to that. I'm not sure it worth trying to "codify a spirit" > (from > > > > > > previous emails), but > > > > > > I think having good advice is helpful to instill the spirit of > > > > goodwill and > > > > > > collaboration > > > > > > into the community. > > > > > > > > > > > > In this regard: > > > > > > > - I propose to create a PR to update the contributing guide > with > > > this > > > > > > > good practices discussed in this thread > > > > > > > > > > > > > > > > > > > +1 > > > > > > > > > > > > - I propose to close https://github.com/apache/polaris/pull/840 > in > > > > > > > favor of "soft rule" (we quickly discuss about owner review > with > > > > > > > > > > > > > > > > > > > +1 > > > > > > > > > > > > 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 > > > > > > > > > > > > > > > > > > +1 > > > > > > > > > > > > Cheers, > > > > > > Dmitri. > > > > > > > > > > > > > > > >
