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

[ci] Add linter for PR title and body #12367

Merged
merged 2 commits into from
Aug 31, 2022
Merged

[ci] Add linter for PR title and body #12367

merged 2 commits into from
Aug 31, 2022

Conversation

driazati
Copy link
Member

@driazati driazati commented Aug 10, 2022

This adds a lint step that checks the PR title and body with some of the basic checks from apache/tvm-rfcs#88. This makes it so we can enforce the decided rules without having to rely on automated fix-ups, though for certain things those would be ideal in the future. The rules implemented here are not complete as the RFC is not fully merged but this is just supposed to be a starting point with some of the basic rules, namely that a title and body should be present. A follow up PR will enforce that there are no @ tags once #12637 is resolved.

cc @Mousius @areusch @gigiblender @gromero @leandron

@driazati driazati force-pushed the pr_linter branch 7 times, most recently from 1437ca6 to 68cfdc1 Compare August 10, 2022 21:53
@github-actions
Copy link
Contributor

Built docs for commit 68cfdc1 can be found here.

Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

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

awesome!

tests/scripts/check_pr.py Outdated Show resolved Hide resolved
tests/scripts/check_pr.py Outdated Show resolved Hide resolved
tests/scripts/check_pr.py Outdated Show resolved Hide resolved
tests/scripts/check_pr.py Outdated Show resolved Hide resolved
tests/scripts/check_pr.py Outdated Show resolved Hide resolved
@driazati driazati force-pushed the pr_linter branch 2 times, most recently from 2a31748 to da98213 Compare August 11, 2022 22:56
@driazati driazati marked this pull request as ready for review August 11, 2022 22:56
@driazati driazati marked this pull request as draft August 11, 2022 22:56
@github-actions github-actions bot requested a review from Mousius August 25, 2022 17:58
@github-actions github-actions bot requested a review from gromero August 30, 2022 22:22
This got out of date after merging apache#12178

Co-authored-by: driazati <[email protected]>
@driazati driazati marked this pull request as ready for review August 30, 2022 22:25
@github-actions github-actions bot requested a review from leandron August 30, 2022 22:26
@github-actions
Copy link
Contributor

No additional skipped tests found in this branch for commit 24b4440.

@areusch areusch merged commit a399e6c into apache:main Aug 31, 2022
@vinx13
Copy link
Member

vinx13 commented Sep 1, 2022

hey @driazati thanks for adding this linter. I found it not throwing meaningful CI failure message when PR body is missing (empty). Here are the CI error message:

[2022-09-01T20:59:13.733Z] Traceback (most recent call last):
[2022-09-01T20:59:13.733Z]   File "ci/scripts/check_pr.py", line 136, in <module>
[2022-09-01T20:59:13.733Z]     body = body.strip()
[2022-09-01T20:59:13.733Z] AttributeError: 'NoneType' object has no attribute 'strip'

Shall we improve it?

@driazati
Copy link
Member Author

driazati commented Sep 1, 2022

thanks for flagging that! #12680 should fix it

xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
* [skip ci][ci] Fix Jenkinsfile (apache#12387)

This got out of date after merging apache#12178

Co-authored-by: driazati <[email protected]>

* Address comments

Co-authored-by: driazati <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants