spark-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sean Owen <so...@cloudera.com>
Subject Re: auto closing pull requests that have been inactive > 30 days?
Date Tue, 19 Apr 2016 08:56:24 GMT
I support this. We used to do this, right? Anecdotally, from watching
the stream most days, most stale PRs are, in descending order of
frequency:

1. Probably not a good change, and not looked at (as a result)
2. Abandoned by submitter at some stage
3. Not an important change, not so bad, not really reviewed
4. A good change that needs review

Whether your PR is #1 or #4 is a matter of perspective. But, I
disagree with the tacit assumption that we're mostly talking about
good PRs being closed because nobody could be bothered; #4 is, I
think, well under 10%.

So generating reports and warnings etc don't seem to address that.
Closing merely means "at the moment there's not a reason to expect
this to proceed, but that could change". Unlike JIRA we don't have
more nuanced resolutions like "WontFix" vs "Later". Welcome the
submitter to reopen if they really think it should be kept alive in
good faith.

As for always stating a close reason: well, a lot of PRs are simply
not very good code, or features that just don't look that useful
relative to their cost. Is it more polite to soft-close or honestly
say "I don't think this is worth adding"?

There is a carrying cost to not doing this. Right now being "Open" is
fairly meaningless, and I've long since stopped bothering reviewing
the backlog of open PRs since it's noise, instead sifting for good new
ones to fast-track.


I agree with comments here that suggest more can be pushed back on the
contributors. We've started with a campaign to get people to read
https://cwiki.apache.org/confluence/display/SPARK/Contributing+to+Spark
first, which would solve a lot of the "mystery pull request" problems
if followed: no real description, no test, no real problem statement.

Put another way, any contribution that is clearly explained, cleanly
implemented, and makes a good case why its pros outweigh its cons, is
pretty consistently reviewed and quickly. Pushing on contributors to
do these things won't harm good contributions, which already do these
things; it'll make it harder for bad contributions to distract from
them.

And I think the effect of a change like this is, in the main, to push
back mostly on less good contributions.

On Mon, Apr 18, 2016 at 8:02 PM, Reynold Xin <rxin@databricks.com> wrote:
> We have hit a new high in open pull requests: 469 today. While we can
> certainly get more review bandwidth, many of these are old and still open
> for other reasons. Some are stale because the original authors have become
> busy and inactive, and some others are stale because the committers are not
> sure whether the patch would be useful, but have not rejected the patch
> explicitly. We can cut down the signal to noise ratio by closing pull
> requests that have been inactive for greater than 30 days, with a nice
> message. I just checked and this would close ~ half of the pull requests.
>
> For example:
>
> "Thank you for creating this pull request. Since this pull request has been
> inactive for 30 days, we are automatically closing it. Closing the pull
> request does not remove it from history and will retain all the diff and
> review comments. If you have the bandwidth and would like to continue
> pushing this forward, please reopen it. Thanks again!"
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@spark.apache.org
For additional commands, e-mail: dev-help@spark.apache.org


Mime
View raw message