spark-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Matthew Powers <matthewkevinpow...@gmail.com>
Subject Re: Auto-closing PRs or How to get reviewers' attention
Date Tue, 23 Feb 2021 15:46:30 GMT
Enrico - thanks for sharing your experience.

I recently got a couple of PRs merged and my experience was different.  I
got lots of feedback from several maintainers (thank you very much!).

Can't speak to your PRs specifically, but can give the general advice that
pivoting code based on maintainer feedback is probably the easiest way to
get stuff merged.

I initially added an add_hours function to org.apache.spark.sql.functions
and it seemed pretty clear that the maintainers weren't the biggest fans
and were more in favor of a make_interval function.  I proactively closed
my own add_hours PR and pushed forward make_interval instead.

In hindsight, add_hours would have been a bad addition to the API and I'm
glad it got rejected.  For big, mature projects like Spark, it's more
important for maintainers to reject stuff than add new functionality.
Software bloat is the main risk for Spark.

I'm of the opinion that the auto-closing PR feature is working well.  Spark
maintainers have a difficult job of having to say "no" and disappoint
people a lot.  Auto closing is a great way to indirectly communicate the
"no" in a way that's more psychologically palatable for both the maintainer
and the committer.

On Tue, Feb 23, 2021 at 9:13 AM Sean Owen <srowen@gmail.com> wrote:

> Yes, committers are added regularly. I don't think that changes the
> situation for most PRs that perhaps just aren't suitable to merge.
> Again the best thing you can do is make it as easy to merge as possible
> and find people who have touched the code for review. This often works out.
>
> On Tue, Feb 23, 2021 at 4:06 AM Enrico Minack <mail@enrico.minack.dev>
> wrote:
>
>> Am 18.02.21 um 16:34 schrieb Sean Owen:
>> > One other aspect is that a committer is taking some degree of
>> > responsibility for merging a change, so the ask is more than just a
>> > few minutes of eyeballing. If it breaks something the merger pretty
>> > much owns resolving it, and, the whole project owns any consequence of
>> > the change for the future.
>>
>> I think this explains the hesitation pretty well: Committers take
>> ownership of the change. It is understandable that PRs then have to be
>> very convincing with low risk/benefit ratio.
>>
>> Are there plans or initiatives to proactively widen the base of
>> committers to mitigate the current situation?
>>
>> Enrico
>>
>>

Mime
View raw message