Skip to content

Commit d5db95b

Browse files
committed
ci: Use our own pr_title_lint.py instead of NPM commitlint [citest_skip]
There are a few problems with the NPM commitlint * NPM is problematic * Having to install NPM and commitlint and dependencies is slow and heavyweight * commitlint is overkill for our simple use case We have our own pr_title_lint.py which is small and works exactly for our use case. Enhance coderabbit checks for PRs and test cleanup Signed-off-by: Rich Megginson <rmeggins@redhat.com>
1 parent d5998c9 commit d5db95b

3 files changed

Lines changed: 40 additions & 147 deletions

File tree

.coderabbit.yaml

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,14 @@ reviews:
3939
mode: "warning"
4040
requirements: |
4141
PR title MUST follow Conventional Commits format:
42-
- Format: <type>: <description> or <type>!: <description> for breaking changes
43-
- Valid types: Refer to the 'type-enum' rule in .commitlintrc.js file for the complete list of allowed types
42+
- Format: <required type><optional (scope)><optional !>: <required description>
43+
- Valid types: Refer to https://github.com/linux-system-roles/auto-maintenance/blob/main/pr_title_lint.py#L23
4444
- Examples:
4545
- "feat: Add backup functionality"
4646
- "fix: Correct OSTree package installation"
4747
- "fix!: Remove deprecated variable (breaking change)"
48+
- "chore(formatting): Fix indentation in my_module.py"
49+
- "refactor(python)!: Use new python library thispy which has breaking api changes"
4850
4951
custom_checks:
5052
- mode: "warning"
@@ -55,6 +57,8 @@ reviews:
5557
- Must contain "Reason:" section explaining why the change was needed
5658
- Must contain "Result:" section describing the outcome or impact
5759
- Can contain optional "Issue Tracker Tickets (Jira or BZ if any):" section
60+
- Must contain "Signed-off-by:" section with your name and email address - use git commit -s
61+
- Can contain optional "Assisted-by:" section with name of the AI coding assistant and models used
5862
5963
Example:
6064
```
@@ -65,6 +69,34 @@ reviews:
6569
Result: Users can now set firewall_secure_logging: false for debugging while maintaining secure defaults.
6670
6771
Issue Tracker Tickets (Jira or BZ if any): RHEL-12345
72+
73+
Signed-off-by: John Doe john.doe@example.com
74+
75+
Assisted-by: Fish 4.3 using model Swim 6.2
76+
```
77+
78+
For PRs that are bug fixes, you can use the following template:
79+
- Must contain "Cause:" section explaining the root cause of the bug
80+
- Must contain "Consequences:" section explaining the impact of the bug and how it appears to users
81+
- Must contain "Fix:" section explaining the fix for the bug and how it fixes the bug
82+
- Must contain "Result:" section describing the outcome or impact of the fix
83+
- Can contain optional "Issue Tracker Tickets (Jira or BZ if any):" section
84+
- Must contain "Signed-off-by:" section with your name and email address - use git commit -s
85+
- Can contain optional "Assisted-by:" section with name of the AI coding assistant and models used
86+
87+
Example:
88+
```
89+
Cause: The variable rolename_user_name was being checked for a `none` value but was not being checked for string length greater than 0 if a string.
90+
91+
Consequences: The role allowed empty user names to be configured which caused the daemon to report "User not found".
92+
93+
Fix: The variable rolename_user_name is now checked for string length if not `none`, and will report an error in that case if the length is 0.
94+
95+
Result: The role will not allow the user to provide an empty user name and will report an error if the rolename_user_name length is 0.
96+
97+
Signed-off-by: John Doe john.doe@example.com
98+
99+
Assisted-by: Fish 4.3 using model Swim 6.2
68100
```
69101
70102
path_instructions:
@@ -180,6 +212,8 @@ reviews:
180212
- Tests should verify both success and failure scenarios
181213
- Use assert module to verify expected state after role execution
182214
- Include cleanup tasks to ensure tests are rerunnable
215+
- Tests should be run in a block with an always section that runs the test cleanup to ensure that the cleanup is always run.
216+
- The cleanup tasks should be tagged with `tests::cleanup` so that the cleanup can be skipped for debug purposes.
183217
- Tests should be idempotent - running twice should not cause failures
184218
- Example verification:
185219
```yaml

.commitlintrc.js

Lines changed: 0 additions & 141 deletions
This file was deleted.

.github/workflows/pr-title-lint.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,11 @@ jobs:
2222
with:
2323
fetch-depth: 0
2424

25-
- name: Install conventional-commit linter
26-
run: npm install @commitlint/config-conventional @commitlint/cli
25+
- name: Install pr_title_lint.py
26+
run: curl -o pr_title_lint.py https://raw.githubusercontent.com/linux-system-roles/auto-maintenance/main/pr_title_lint.py
2727

28-
- name: Run commitlint on PR title
28+
- name: Run pr_title_lint.py on PR title
2929
env:
3030
PR_TITLE: ${{ github.event.pull_request.title }}
3131
# Echo from env variable to avoid bash errors with extra characters
32-
run: echo "$PR_TITLE" | npx commitlint --verbose
32+
run: python3 pr_title_lint.py "${PR_TITLE}"

0 commit comments

Comments
 (0)