Btw, there are two more PRs which got LGTM by a SS contributor but fail to get attention from committers. They're 6+ months old. Could you help reviewing this as well, or do you all think 6 months of time range + LGTM from an SS contributor is enough to go ahead?
https://github.com/apache/spark/pull/27649 https://github.com/apache/spark/pull/28363 These are under 100 lines of changes per each, and not invasive. On Sat, Nov 28, 2020 at 11:34 AM Jungtaek Lim <kabhwan.opensou...@gmail.com> wrote: > Thanks for providing valuable feedback. Appreciate it. Sorry I haven't had > time to reply to this in time (was OoO this week). > > I'm also in favor of "review then commit", I haven't been a "perfect" guy > making no mistake (probably that justifies me as a human being), hence the > review process is a critical one I really would like to go through in any > way. The problem is, it's less likely I could get attention for my SS PRs > from "committers". Hopefully there are a couple of contributors in the SS > area, so getting reviewed by itself is a bit easier than before, but in any > way I couldn't get a finalized review and go merging. > > This PR is actually an easy case, small enough and not invasive. I have > planned major features most likely to bring design changes, which cannot go > without attention from committers. I'd probably need committers for both > design & code review - I'm a bit worried that such major change could be > achieved with current situation. > (This may not be resolved even with the time limit... If the community has > a faith to me then I could bravely just go with time limit, but, is this > something I can avoid ending up being blamed?) > > Probably the better resolution is filling enough SS committers. There > should be some ways to do this, existing committers expert of SS area come > back (which isn't something we can control), committers expert of other > area jump in and help the area (which isn't also something we can control), > or inviting new committer(s) for SS area. Anything would be fine for me. > > > On Tue, Nov 24, 2020 at 2:46 AM Sean Owen <sro...@gmail.com> wrote: > >> Yes, agree, and that time limit is probably a lot shorter than 1.5 years. >> I think these ultimately come down to judgment, and am affirming the >> judgment that this amounts to 'reviewed'. >> >> On Mon, Nov 23, 2020 at 11:40 AM Ryan Blue <rb...@netflix.com> wrote: >> >>> I'll go take a look. >>> >>> While I would generally agree with Sean that it would be appropriate in >>> this case to commit, I'm very hesitant to set that precedent. I'd prefer to >>> stick with "review then commit" and, if needed, relax that constraint for >>> parts of the project that can't get reviewers for a certain period of time. >>> We did that in another community where there weren't many reviewers and we >>> wanted to get more people involved, but we put a time limit on it and set >>> expectations to prevent any perception of abuse. I would support doing that >>> in SS. >>> >>> Thanks for being so patient on that PR. I'm sorry that you had to wait >>> so long. >>> >>> On Mon, Nov 23, 2020 at 7:11 AM Sean Owen <sro...@gmail.com> wrote: >>> >>>> I don't see any objections on that thread. You're a committer and have >>>> reviews from other knowledgeable people in this area. Do you have any >>>> reason to believe it's controversial, like, changes semantics or APIs? Were >>>> there related discussions elsewhere that expressed any concern? >>>> >>>> From a glance, OK it's introducing a new idea of state schema and >>>> validation; would it conflict with any other possible approaches, have any >>>> limits if this is enshrined as supported functionality? There's always some >>>> cost to introducing yet more code to support, but, this doesn't look >>>> intrusive or large. >>>> >>>> The "don't review your own PR" idea isn't hard-and-fast. I don't think >>>> anyone needs to block for anything like this long if you have other capable >>>> reviews and you are a committer, if you don't see that it impacts other >>>> code meaningfully in a way that really demands review from others, and in >>>> good faith judge that it is worthwhile. I think you are the one de facto >>>> expert on that code and indeed you can't block yourself for 1.5 years or >>>> else nothing substantial would happen. >>>> >>>> >>>> >>>> On Mon, Nov 23, 2020 at 1:18 AM Jungtaek Lim < >>>> kabhwan.opensou...@gmail.com> wrote: >>>> >>>>> Hi devs, >>>>> >>>>> I have been struggling to find reviewers who are committers, to get my >>>>> PR [1] for SPARK-27237 [2] reviewed. The PR was submitted on Mar. 2019 >>>>> (1.5 >>>>> years ago), and somehow it got two approvals from contributors working on >>>>> the SS area, but still doesn't get any committers' traction to review. >>>>> (I can review others' SS PRs and I'm trying to unblock other SS area >>>>> contributors, but I can't self review my SS PRs. Not sure it's technically >>>>> possible, but fully sure it's not encouraged.) >>>>> >>>>> Could I please ask help to unblock this before feature freeze for >>>>> Spark 3.1 is happening? Submitted 1.5 years ago and continues struggling >>>>> for including it in Spark 3.2 (another half of a year) doesn't make sense >>>>> to me. >>>>> >>>>> In addition, is there a way to unblock me to work for meaningful >>>>> features instead of being stuck with small improvements? I have something >>>>> in my backlog but I'd rather not want to continue struggling with new PRs. >>>>> >>>>> Thanks, >>>>> Jungtaek Lim (HeartSaVioR) >>>>> >>>>> 1. https://github.com/apache/spark/pull/24173 >>>>> 2. https://issues.apache.org/jira/browse/SPARK-27237 >>>>> >>>> >>> >>> -- >>> Ryan Blue >>> Software Engineer >>> Netflix >>> >>
