Skip to content

Commit ec7e60c

Browse files
mrpollomrpollo
committed
ci(workflows): add commit message and PR title quality checks
Add CI enforcement of conventional commit format for PR titles and commit messages. Includes three Python scripts under Tools/ci/: - conventional_commits.py: shared parsing/validation library - check_pr_title.py: validates PR title format, suggests fixes - check_commit_messages.py: checks commits for blocking errors (fixup/squash/WIP leftovers) and advisory warnings (review-response, formatter-only commits) The workflow (.github/workflows/commit_checks.yml) posts concise GitHub PR comments with actionable suggestions and auto-removes them once issues are resolved. Also updates CONTRIBUTING.md and docs with the conventional commits convention. Signed-off-by: Ramon Roche <mrpollo@gmail.com>
1 parent b5deafd commit ec7e60c

File tree

8 files changed

+1030
-42
lines changed

8 files changed

+1030
-42
lines changed

‎.github/workflows/commit_checks.yml‎

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

‎CONTRIBUTING.md‎

Lines changed: 124 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,150 @@
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 [conventional commits](https://www.conventionalcommits.org/) for all commit messages and PR titles.
2424

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

2727
```
28-
Change how the attitude controller works
28+
type(scope): short description of the change
29+
```
30+
31+
| Part | Rule |
32+
|------|------|
33+
| **type** | Category of change (see types table below) |
34+
| **scope** | The module, driver, board, or area of PX4 affected |
35+
| **`!`** (optional) | Append before `:` to mark a breaking change |
36+
| **description** | What the change does, at least 5 characters, written in imperative form |
37+
38+
### Types
39+
40+
| Type | Description |
41+
|------|-------------|
42+
| `feat` | A new feature |
43+
| `fix` | A bug fix |
44+
| `docs` | Documentation only changes |
45+
| `style` | Formatting, whitespace, no code change |
46+
| `refactor` | Code change that neither fixes a bug nor adds a feature |
47+
| `perf` | Performance improvement |
48+
| `test` | Adding or correcting tests |
49+
| `build` | Build system or external dependencies |
50+
| `ci` | CI configuration files and scripts |
51+
| `chore` | Other changes that don't modify src or test files |
52+
| `revert` | Reverts a previous commit |
53+
54+
### Scopes
55+
56+
The scope identifies which part of PX4 is affected. Common scopes:
57+
58+
| Scope | Area |
59+
|-------|------|
60+
| `ekf2` | Extended Kalman Filter (state estimation) |
61+
| `mavlink` | MAVLink messaging protocol |
62+
| `commander` | Commander and mode management |
63+
| `navigator` | Mission, RTL, Land, and other navigation modes |
64+
| `sensors` | Sensor drivers and processing |
65+
| `drivers` | Hardware drivers |
66+
| `boards/px4_fmu-v6x` | Board-specific changes (use the board name) |
67+
| `mc_att_control` | Multicopter attitude control |
68+
| `mc_pos_control` | Multicopter position control |
69+
| `fw_att_control` | Fixed-wing attitude control |
70+
| `vtol` | VTOL-specific logic |
71+
| `actuators` | Mixer and actuator output |
72+
| `battery` | Battery monitoring and estimation |
73+
| `logger` | On-board logging |
74+
| `param` | Parameter system |
75+
| `simulation` | SITL, Gazebo, SIH |
76+
| `ci` | Continuous integration and workflows |
77+
| `docs` | Documentation |
78+
| `build` | CMake, toolchain, build system |
79+
| `uorb` | Inter-module messaging |
80+
81+
For changes spanning multiple subsystems, use the primary one affected. Look at the directory path of the files you changed to find the right scope: `src/modules/ekf2/` uses `ekf2`, `src/drivers/imu/` uses `drivers/imu`, `.github/workflows/` uses `ci`.
82+
83+
### Breaking changes
84+
85+
Append `!` before the colon to indicate a breaking change:
86+
87+
```
88+
feat(ekf2)!: remove deprecated height fusion API
89+
```
90+
91+
### Good commit messages
92+
93+
```
94+
feat(ekf2): add height fusion timeout
95+
fix(mavlink): correct BATTERY_STATUS_V2 parsing
96+
refactor(navigator): simplify RTL altitude logic
97+
ci(workflows): migrate to reusable workflows
98+
docs(ekf2): update tuning guide
99+
feat(boards/px4_fmu-v6x)!: remove deprecated driver API
100+
perf(mc_rate_control): reduce loop latency
101+
```
102+
103+
### Commits to avoid
104+
105+
These will be flagged by CI and should be squashed or reworded before merging:
106+
107+
```
108+
fix # too vague, no type or scope
109+
update # too vague, no type or scope
110+
ekf2: fix something # missing type prefix
111+
apply suggestions from code review # squash into parent commit
112+
do make format # squash into parent commit
113+
WIP: trying something # not ready for main
114+
oops # not descriptive
115+
```
116+
117+
### PR titles
118+
119+
The PR title follows the same `type(scope): description` format. This is enforced by CI and is especially important because the PR title becomes the commit message when a PR is squash-merged.
120+
121+
### Merge policy
122+
123+
Commits should be atomic and independently revertable. Squash at reviewer discretion for obvious cases (multiple WIP commits, messy review-response history). When your commits are clean and logical, they will be preserved as individual commits on `main`.
124+
125+
### Cleaning up commits
126+
127+
If CI flags your commit messages, you can fix them with an interactive rebase:
29128

30-
- Fixes rate feed forward
31-
- Allows a local body rate override
129+
```bash
130+
# Squash all commits into one:
131+
git rebase -i HEAD~N # replace N with the number of commits
132+
# mark all commits except the first as 'squash' or 'fixup'
133+
# reword the remaining commit to follow the format
134+
git push --force-with-lease
32135

33-
Fixes issue #123
136+
# Or reword specific commits:
137+
git rebase -i HEAD~N
138+
# mark the bad commits as 'reword'
139+
git push --force-with-lease
34140
```
35141

36-
### Test your changes
142+
## Test your changes
37143

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.
144+
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.
39145

40-
### Push your changes
146+
## Push your changes
41147

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

44150
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)