Skip to content
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

[SPARK-28857][INFRA] Clean up the comments of PR template during merging #25564

Closed
wants to merge 1 commit into from
Closed

[SPARK-28857][INFRA] Clean up the comments of PR template during merging #25564

wants to merge 1 commit into from

Conversation

dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Aug 23, 2019

What changes were proposed in this pull request?

This PR aims to clean up the commit logs by removing the comments of our PR template.

Why are the changes needed?

Apache Spark PR template has comments. Sometime we forget to clean up them because GitHub hides them nicely. It would be great if we clean up this. Otherwise, this makes the commit logs too verbose. (There are a few commits already.)

Does this PR introduce any user-facing change?

No. (only for committers)

How was this patch tested?

Manually with Python2/Python3.

@dongjoon-hyun
Copy link
Member Author

Could you review this PR, @gatorsmile , @cloud-fan , @HyukjinKwon ?

@@ -495,7 +495,24 @@ def main():
else:
title = pr["title"]

body = pr["body"]
modified_body = re.sub(re.compile(r'<!--[^>]*-->\n?', re.DOTALL), '', pr["body"]).lstrip()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lstrip() is used to remove the first PR template comments and new lines. strip() has false positive because it will modify all PR description (which doesn't have comments).

@cloud-fan
Copy link
Contributor

thanks for fixing it! LGTM

@dongjoon-hyun
Copy link
Member Author

Thank you for review and approval, @cloud-fan !

@SparkQA
Copy link

SparkQA commented Aug 23, 2019

Test build #109616 has finished for PR 25564 at commit 0b0e02e.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor

retest this please

@HyukjinKwon
Copy link
Member

Merged to master given that it's tested.

PR builders won't do anything with this script except linter .. :-)..

@SparkQA
Copy link

SparkQA commented Aug 23, 2019

Test build #109627 has finished for PR 25564 at commit 0b0e02e.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun
Copy link
Member Author

Thank you, @cloud-fan , @srowen , @HyukjinKwon !

@dongjoon-hyun dongjoon-hyun deleted the SPARK-28857 branch August 23, 2019 14:39
@dongjoon-hyun
Copy link
Member Author

I'll cherry-pick this to branch-2.4, too. I noticed that the script in branch-2.4 is also used sometime.

dongjoon-hyun added a commit that referenced this pull request Sep 18, 2019
### What changes were proposed in this pull request?

This PR aims to clean up the commit logs by removing the comments of our PR template.

### Why are the changes needed?

Apache Spark PR template has comments. Sometime we forget to clean up them because GitHub hides them nicely. It would be great if we clean up this. Otherwise, this makes the commit logs too verbose. (There are a few commits already.)

### Does this PR introduce any user-facing change?

No. (only for committers)

### How was this patch tested?

Manually with Python2/Python3.

Closes #25564 from dongjoon-hyun/SPARK-28857.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: HyukjinKwon <[email protected]>
(cherry picked from commit 1fd7f29)
Signed-off-by: Dongjoon Hyun <[email protected]>
@dongjoon-hyun
Copy link
Member Author

cc @viirya

@viirya
Copy link
Member

viirya commented Sep 18, 2019

@dongjoon-hyun Yeah, sounds good. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants