spark-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jungtaek Lim <kabhwan.opensou...@gmail.com>
Subject Re: Seeking committers' help to review on SS PR
Date Sat, 28 Nov 2020 02:54:48 GMT
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.opensource@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 <srowen@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 <rblue@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 <srowen@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.opensource@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
>>>
>>

Mime
View raw message