-
Notifications
You must be signed in to change notification settings - Fork 28.7k
[SPARK-28616][INFRA] Improve merge-spark-pr script to warn WIP PRs and strip trailing dots #25356
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…d strip trailing dots
Test build #108637 has finished for PR 25356 at commit
|
retest this please |
All test passed and the CRAN check failed because they seems to start to enforce R 3.4 and the above.
I'll merge this PR since this is not related to the Spark code. |
Thank you always for review and approval, @HyukjinKwon ! |
Oops. I didn't read your |
I think that's fine. |
Test FAILed. |
I'll file a JIRA for CRAN failure since we need to upgrade R in Jenkins. |
Thanks.
This error seems to be newer one. cc @viirya FYI. |
Ya. I'm checking the other commits and found that #24560 passed 4 hours ago without this problem. Maybe, it might be a transient issue. I'll keep monitoring~ |
Looks different to previous CRAN error. Was it happened again? |
Yes, seems so - #25363 (comment) |
looks intermittent though .. |
I'll backport this to |
…d strip trailing dots ## What changes were proposed in this pull request? This PR aims to improve the `merge-spark-pr` script in the following two ways. 1. `[WIP]` is useful when we show that a PR is not ready for merge. Apache Spark allows merging `WIP` PRs. However, sometime, we accidentally forgot to clean up the title for the completed PRs. We had better warn once more during merging stage and get a confirmation from the committers. 2. We have two kinds of PR titles in terms of the ending period. This PR aims to remove the trailing `dot` since the shorter is the better in the commit title. Also, the PR titles without the trailing `dot` is dominant in the Apache Spark commit logs. ``` $ git log --oneline | grep '[.]$' | wc -l 4090 $ git log --oneline | grep '[^.]$' | wc -l 20747 ``` ## How was this patch tested? Manual. ``` $ dev/merge_spark_pr.py git rev-parse --abbrev-ref HEAD Which pull request would you like to merge? (e.g. 34): 25157 The PR title has `[WIP]`: [WIP][SPARK-28396][SQL] Add PathCatalog for data source V2 Continue? (y/n): ``` ``` $ dev/merge_spark_pr.py git rev-parse --abbrev-ref HEAD Which pull request would you like to merge? (e.g. 34): 25304 I've re-written the title as follows to match the standard format: Original: [SPARK-28570][CORE][SHUFFLE] Make UnsafeShuffleWriter use the new API. Modified: [SPARK-28570][CORE][SHUFFLE] Make UnsafeShuffleWriter use the new API Would you like to use the modified title? (y/n): ``` Closes #25356 from dongjoon-hyun/SPARK-28616. Authored-by: Dongjoon Hyun <dhyun@apple.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit ae08387) Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
What changes were proposed in this pull request?
This PR aims to improve the
merge-spark-pr
script in the following two ways.[WIP]
is useful when we show that a PR is not ready for merge. Apache Spark allows mergingWIP
PRs. However, sometime, we accidentally forgot to clean up the title for the completed PRs. We had better warn once more during merging stage and get a confirmation from the committers.dot
since the shorter is the better in the commit title. Also, the PR titles without the trailingdot
is dominant in the Apache Spark commit logs.How was this patch tested?
Manual.