Skip to content

Commit c13f937

Browse files
mrpollomrpollo
committed
CI: add commit message and PR title quality checks
Add automated validation of PR titles and commit messages against PX4's subsystem: description convention. Three CI jobs run on every pull request: - pr-title: blocks PRs with titles missing the subsystem prefix - commit-messages: blocks fixup/WIP/throwaway commits, warns on missing prefixes and review-response messages - pr-body: advisory warning for empty PR descriptions For non-fork PRs, posts rich markdown comments with suggested fixes, rebase instructions, and convention docs. Comments auto-remove when issues are resolved. Fork PRs get guidance in CI logs with links to CONTRIBUTING.md. Also updates CONTRIBUTING.md with the full commit message convention reference (format table, subsystem examples, rebase instructions). Signed-off-by: Ramon Roche <mrpollo@gmail.com>
1 parent c424edd commit c13f937

File tree

5 files changed

+1026
-18
lines changed

5 files changed

+1026
-18
lines changed

‎.github/workflows/commit_checks.yml‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
name: Commit Quality
2+
3+
on:
4+
pull_request:
5+
types: [opened, edited, synchronize, reopened]
6+
7+
permissions:
8+
contents: read
9+
pull-requests: write
10+
11+
concurrency:
12+
group: ${{ github.workflow }}-${{ github.ref }}
13+
cancel-in-progress: true
14+
15+
env:
16+
PR_NUMBER: ${{ github.event.pull_request.number }}
17+
IS_FORK: ${{ github.event.pull_request.head.repo.full_name != github.repository }}
18+
19+
jobs:
20+
pr-title:
21+
name: PR Title
22+
runs-on: ubuntu-latest
23+
steps:
24+
- name: Checkout
25+
uses: actions/checkout@v4
26+
with:
27+
sparse-checkout: Tools/ci
28+
fetch-depth: 1
29+
30+
- name: Check PR title
31+
id: check
32+
run: |
33+
python3 Tools/ci/check_pr_title.py "${{ github.event.pull_request.title }}" --markdown > comment.md 2>/dev/null && rc=0 || rc=$?
34+
echo "exit_code=$rc" >> "$GITHUB_OUTPUT"
35+
36+
- name: Post or clear comment
37+
if: env.IS_FORK == 'false'
38+
env:
39+
GH_TOKEN: ${{ github.token }}
40+
run: |
41+
if [ "${{ steps.check.outputs.exit_code }}" != "0" ]; then
42+
python3 Tools/ci/pr_comment.py --marker pr-title --pr "$PR_NUMBER" --result fail < comment.md
43+
else
44+
python3 Tools/ci/pr_comment.py --marker pr-title --pr "$PR_NUMBER" --result pass
45+
fi
46+
47+
- name: Result
48+
if: always()
49+
run: |
50+
if [ "${{ steps.check.outputs.exit_code }}" != "0" ]; then
51+
# Show the error in the step that fails so it's visible
52+
python3 Tools/ci/check_pr_title.py "${{ github.event.pull_request.title }}" 2>&1 || true
53+
echo ""
54+
echo "::error::PR title does not follow the required format. See output above."
55+
exit 1
56+
else
57+
echo "PR title is valid."
58+
fi
59+
60+
commit-messages:
61+
name: Commit Messages
62+
runs-on: ubuntu-latest
63+
steps:
64+
- name: Checkout
65+
uses: actions/checkout@v4
66+
with:
67+
sparse-checkout: Tools/ci
68+
fetch-depth: 1
69+
70+
- name: Check commit messages
71+
id: check
72+
env:
73+
GH_TOKEN: ${{ github.token }}
74+
run: |
75+
commits=$(gh api \
76+
"repos/${{ github.repository }}/pulls/${PR_NUMBER}/commits?per_page=100")
77+
echo "$commits" | python3 Tools/ci/check_commit_messages.py --markdown > comment.md && rc=0 || rc=$?
78+
# Store commits for the result step
79+
echo "$commits" > commits.json
80+
echo "exit_code=$rc" >> "$GITHUB_OUTPUT"
81+
# Check for warnings (non-empty markdown on exit 0)
82+
if [ "$rc" -eq 0 ] && [ -s comment.md ]; then
83+
echo "has_warnings=true" >> "$GITHUB_OUTPUT"
84+
else
85+
echo "has_warnings=false" >> "$GITHUB_OUTPUT"
86+
fi
87+
88+
- name: Post or clear comment
89+
if: env.IS_FORK == 'false'
90+
env:
91+
GH_TOKEN: ${{ github.token }}
92+
run: |
93+
if [ "${{ steps.check.outputs.exit_code }}" != "0" ]; then
94+
python3 Tools/ci/pr_comment.py --marker commit-msgs --pr "$PR_NUMBER" --result fail < comment.md
95+
elif [ "${{ steps.check.outputs.has_warnings }}" == "true" ]; then
96+
python3 Tools/ci/pr_comment.py --marker commit-msgs --pr "$PR_NUMBER" --result warn < comment.md
97+
else
98+
python3 Tools/ci/pr_comment.py --marker commit-msgs --pr "$PR_NUMBER" --result pass
99+
fi
100+
101+
- name: Result
102+
if: always()
103+
run: |
104+
if [ "${{ steps.check.outputs.exit_code }}" != "0" ]; then
105+
# Show the error in the step that fails so it's visible
106+
python3 Tools/ci/check_commit_messages.py < commits.json || true
107+
echo ""
108+
echo "::error::Commit message errors found. See output above."
109+
exit 1
110+
elif [ "${{ steps.check.outputs.has_warnings }}" == "true" ]; then
111+
python3 Tools/ci/check_commit_messages.py < commits.json || true
112+
else
113+
echo "All commit messages are valid."
114+
fi
115+
116+
pr-body:
117+
name: PR Description
118+
runs-on: ubuntu-latest
119+
steps:
120+
- name: Checkout
121+
if: env.IS_FORK == 'false'
122+
uses: actions/checkout@v4
123+
with:
124+
sparse-checkout: Tools/ci
125+
fetch-depth: 1
126+
127+
- name: Check PR body
128+
id: check
129+
env:
130+
PR_BODY: ${{ github.event.pull_request.body }}
131+
run: |
132+
message=""
133+
if [ -z "$PR_BODY" ]; then
134+
message="PR description is empty. Please add a summary of the changes."
135+
echo "::warning::PR description is empty."
136+
else
137+
cleaned=$(echo "$PR_BODY" | sed 's/<!--.*-->//g' | tr -d '[:space:]')
138+
if [ -z "$cleaned" ]; then
139+
message="PR description contains only template comments. Please fill in the details."
140+
echo "::warning::PR description contains only template comments."
141+
fi
142+
fi
143+
echo "message=$message" >> "$GITHUB_OUTPUT"
144+
145+
- name: Post or clear comment
146+
if: env.IS_FORK == 'false'
147+
env:
148+
GH_TOKEN: ${{ github.token }}
149+
run: |
150+
if [ -n "${{ steps.check.outputs.message }}" ]; then
151+
printf '%s\n' \
152+
"## PR Description (advisory)" \
153+
"" \
154+
"This is **not blocking**, but your PR description appears to be empty or incomplete." \
155+
"" \
156+
"${{ steps.check.outputs.message }}" \
157+
"" \
158+
"A good PR description helps reviewers understand what changed and why." \
159+
"" \
160+
"---" \
161+
"*This comment will be automatically removed once the issue is resolved.*" \
162+
| python3 Tools/ci/pr_comment.py --marker pr-body --pr "$PR_NUMBER" --result warn
163+
else
164+
python3 Tools/ci/pr_comment.py --marker pr-body --pr "$PR_NUMBER" --result pass
165+
fi

‎CONTRIBUTING.md‎

Lines changed: 91 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,117 @@
1-
# Contributing to PX4 Firmware
1+
# Contributing to PX4-Autopilot
22

3-
We follow the [Github flow](https://guides.github.com/introduction/flow/) development model.
3+
We follow the [GitHub flow](https://guides.github.com/introduction/flow/) development model.
44

5-
### Fork the project, then clone your repo
5+
## Fork the project, then clone your repo
66

7-
First [fork and clone](https://help.github.com/articles/fork-a-repo) the project project.
7+
First [fork and clone](https://help.github.com/articles/fork-a-repo) the project.
88

9-
### Create a feature branch
9+
## Create a feature branch
1010

11-
*Always* branch off main for new features.
11+
Always branch off `main` for new features.
1212

1313
```
1414
git checkout -b mydescriptivebranchname
1515
```
1616

17-
### Edit and build the code
17+
## Edit and build the code
1818

1919
The [developer guide](https://docs.px4.io/main/en/development/development.html) explains how to set up the development environment on Mac OS, Linux or Windows. Please take note of our [coding style](https://docs.px4.io/main/en/contribute/code.html) when editing files.
2020

21-
### Commit your changes
21+
## Commit message convention
2222

23-
Always write descriptive commit messages and add a fixes or relates note to them with an [issue number](https://github.com/px4/Firmware/issues) (Github will link these then conveniently)
23+
PX4 uses the `subsystem: description` format for all commit messages and PR titles.
2424

25-
**Example:**
25+
### Format
2626

2727
```
28-
Change how the attitude controller works
28+
subsystem: short description of the change
29+
```
30+
31+
| Part | Rule |
32+
|------|------|
33+
| **subsystem** | The module, driver, board, or area of PX4 that the change affects |
34+
| **`:`** + space | Required separator |
35+
| **description** | What the change does, at least 5 characters, written in imperative form |
36+
37+
### Subsystem examples
38+
39+
Common subsystem prefixes used in PX4:
40+
41+
| Subsystem | Area |
42+
|-----------|------|
43+
| `ekf2` | Extended Kalman Filter (state estimation) |
44+
| `mavlink` | MAVLink messaging protocol |
45+
| `navigator` | Mission, RTL, Land, and other navigation modes |
46+
| `sensors` | Sensor drivers and processing |
47+
| `drivers` | Hardware drivers |
48+
| `boards/px4_fmu-v6x` | Board-specific changes (use the board name) |
49+
| `multicopter` | Multicopter-specific control and estimation |
50+
| `fixedwing` | Fixed-wing-specific control and estimation |
51+
| `vtol` | VTOL-specific logic |
52+
| `actuators` | Mixer and actuator output |
53+
| `battery` | Battery monitoring and estimation |
54+
| `logger` | On-board logging |
55+
| `param` | Parameter system |
56+
| `simulation` | SITL, Gazebo, SIH |
57+
| `CI` | Continuous integration and workflows |
58+
| `docs` | Documentation |
59+
| `build` | CMake, toolchain, build system |
60+
| `uORB` | Inter-module messaging |
61+
62+
For changes spanning multiple subsystems, use the primary one affected.
63+
64+
### Good commit messages
65+
66+
```
67+
ekf2: fix height fusion timeout
68+
mavlink: add BATTERY_STATUS_V2 support
69+
boards/px4_fmu-v6x: enable UAVCAN
70+
navigator: fix RTL altitude calculation
71+
CI: migrate to reusable workflows
72+
docs: update EKF tuning guide
73+
```
74+
75+
### Commits to avoid
76+
77+
These will be flagged by CI and should be squashed or reworded before merging:
78+
79+
```
80+
fix # too vague, no subsystem
81+
update # too vague, no subsystem
82+
apply suggestions from code review # squash into parent commit
83+
do make format # squash into parent commit
84+
WIP: trying something # not ready for main
85+
oops # not descriptive
86+
```
87+
88+
### PR titles
89+
90+
The PR title follows the same `subsystem: description` format. This is especially important because the PR title becomes the commit message when a PR is squash-merged.
91+
92+
### Cleaning up commits
93+
94+
If CI flags your commit messages, you can fix them with an interactive rebase:
2995

30-
- Fixes rate feed forward
31-
- Allows a local body rate override
96+
```bash
97+
# Squash all commits into one:
98+
git rebase -i HEAD~N # replace N with the number of commits
99+
# mark all commits except the first as 'squash' or 'fixup'
100+
# reword the remaining commit to follow the format
101+
git push --force-with-lease
32102

33-
Fixes issue #123
103+
# Or reword specific commits:
104+
git rebase -i HEAD~N
105+
# mark the bad commits as 'reword'
106+
git push --force-with-lease
34107
```
35108

36-
### Test your changes
109+
## Test your changes
37110

38-
Since we care about safety, we will regularly ask you for test results. Best is to do a test flight (or bench test where it applies) and upload the logfile from it (on the microSD card in the logs directory) to Google Drive or Dropbox and share the link.
111+
Since we care about safety, we will regularly ask you for test results. Best is to do a test flight (or bench test where it applies) and upload the log file from it (on the microSD card in the logs directory) to Google Drive or Dropbox and share the link.
39112

40-
### Push your changes
113+
## Push your changes
41114

42-
Push changes to your repo and send a [pull request](https://github.com/PX4/Firmware/compare/).
115+
Push changes to your repo and send a [pull request](https://github.com/PX4/PX4-Autopilot/compare/).
43116

44117
Make sure to provide some testing feedback and if possible the link to a flight log file. Upload flight log files to [Flight Review](http://logs.px4.io) and link the resulting report.

0 commit comments

Comments
 (0)