HeartSaVioR edited a comment on issue #26416: [SPARK-29779][CORE] Compact old event log files and cleanup URL: https://github.com/apache/spark/pull/26416#issuecomment-570444806 There's a dilemma makes contributors not easy to try breaking down first; there's a case when the PR cannot be broken down into multiple PRs without allowing that some parts wouldn't change any behavior and requires other parts to make actual changes. (For me, SPARK-29111 is a good example - the patch had been valid before we changed the strategy, but failed to get reviewers for committing as SPARK-29111 itself doesn't do anything.) Reviewers will require full picture of the feature to make sure we don't end up merging only part of feature which will be no-op. Sometimes it's just the full picture (plan), but some other cases reviewers would require PRs implement the feature entirely; some reviewer may wonder how it would integrate with current Spark while reviewing the earlier part, which would be done in later part. To make this be realistic, both contributor and reviewers should have committed to focus driving the feature forward; otherwise someone would need to put unnecessary efforts. That's SPIP/shepherd is for, but that doesn't seem to always guarantee commitment. That's just my 2 cents on contributor's perspective. This PR is good to break down, as we've been spending huge time to review so far and it convinces me as we don't drop out the feature entirely in further review phase. I'll try to break down the PR and ping reviewers. Thanks for the suggestion.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
