Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/10433#issuecomment-166622804
The question is whether this makes the code more 'welcoming' or not, of
course, and I don't think this does. I did not agree that the current logic is
convoluted; I'm talking about your change, and agreeing with your assessment
that relying on `isFinished` to mean "finished but not FINISHED" is funny.
Of course you don't want to spend time on changes that won't be merged.
However, the amount of effort that goes into a change alone doesn't make it a
better change. No, it would never be appropriate to just merge it and then
think later about whether it was worth it.
A lot of my time and others' is spent reviewing changes that won't be
merged, so rejected PRs/JIRAs cost everyone time*. Ideally we'd all know what
kinds of change will get buy-in from people who can merge ahead of time; the
list at
https://cwiki.apache.org/confluence/display/SPARK/Contributing+to+Spark#ContributingtoSpark-PreparingtoContributeCodeChanges
was written to help describe what makes that likely. I'd also look for JIRAs
opened by committers, which are much more likely to have buy-in.
* Well, they have some value: they're a way of communicating about what
won't be accepted and have value if this saves time in the future. I think many
of your PRs have been rejected on similar grounds, so I'd also look to you to
tailor your choices appropriately, rather than push back further. For example,
https://github.com/apache/spark/pull/10432 is also pretty trivial but is also
fixes with no downside that I can see. Although it's not the most important
thing I'd encourage people to work on, myself, that seems OK. Here the issue is
that it seems to do nothing but make the code marginally less clear.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
