-
Notifications
You must be signed in to change notification settings - Fork 15.3k
ci(workflows): add commit message and PR title quality checks #26581
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| name: Commit Quality | ||
|
|
||
| on: | ||
| pull_request: | ||
| types: [opened, edited, synchronize, reopened] | ||
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
| issues: write | ||
|
|
||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.ref }} | ||
| cancel-in-progress: true | ||
|
|
||
| env: | ||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||
| IS_FORK: ${{ github.event.pull_request.head.repo.full_name != github.repository }} | ||
|
|
||
| jobs: | ||
| pr-title: | ||
| name: PR Title | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| sparse-checkout: Tools/ci | ||
| fetch-depth: 1 | ||
|
|
||
| - name: Check PR title | ||
| id: check | ||
| run: | | ||
| python3 Tools/ci/check_pr_title.py "${{ github.event.pull_request.title }}" --markdown-file comment.md && rc=0 || rc=$? | ||
| echo "exit_code=$rc" >> "$GITHUB_OUTPUT" | ||
|
|
||
| - name: Post or clear comment | ||
| if: env.IS_FORK == 'false' | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| run: | | ||
| if [ "${{ steps.check.outputs.exit_code }}" != "0" ]; then | ||
| python3 Tools/ci/pr_comment.py --marker pr-title --pr "$PR_NUMBER" --result fail < comment.md | ||
| else | ||
| python3 Tools/ci/pr_comment.py --marker pr-title --pr "$PR_NUMBER" --result pass | ||
| fi | ||
|
|
||
| - name: Result | ||
| if: steps.check.outputs.exit_code != '0' | ||
| run: | | ||
| echo "::error::PR title does not follow conventional commits format. See the PR comment for details." | ||
| exit 1 | ||
|
|
||
| commit-messages: | ||
| name: Commit Messages | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| sparse-checkout: Tools/ci | ||
| fetch-depth: 1 | ||
|
|
||
| - name: Check commit messages | ||
| id: check | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| run: | | ||
| gh api \ | ||
| "repos/${{ github.repository }}/pulls/${PR_NUMBER}/commits?per_page=100" \ | ||
| | python3 Tools/ci/check_commit_messages.py --markdown-file comment.md && rc=0 || rc=$? | ||
| echo "exit_code=$rc" >> "$GITHUB_OUTPUT" | ||
| # Check for warnings (non-empty markdown on exit 0) | ||
| if [ "$rc" -eq 0 ] && [ -s comment.md ]; then | ||
| echo "has_warnings=true" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "has_warnings=false" >> "$GITHUB_OUTPUT" | ||
| fi | ||
|
|
||
| - name: Post or clear comment | ||
| if: env.IS_FORK == 'false' | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| run: | | ||
| if [ "${{ steps.check.outputs.exit_code }}" != "0" ]; then | ||
| python3 Tools/ci/pr_comment.py --marker commit-msgs --pr "$PR_NUMBER" --result fail < comment.md | ||
| elif [ "${{ steps.check.outputs.has_warnings }}" == "true" ]; then | ||
| python3 Tools/ci/pr_comment.py --marker commit-msgs --pr "$PR_NUMBER" --result warn < comment.md | ||
| else | ||
| python3 Tools/ci/pr_comment.py --marker commit-msgs --pr "$PR_NUMBER" --result pass | ||
| fi | ||
|
|
||
| - name: Result | ||
| if: steps.check.outputs.exit_code != '0' | ||
| run: | | ||
| echo "::error::Commit message errors found. See the PR comment for details." | ||
| exit 1 | ||
|
|
||
| pr-body: | ||
|
|
||
| name: PR Description | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout | ||
| if: env.IS_FORK == 'false' | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| sparse-checkout: Tools/ci | ||
| fetch-depth: 1 | ||
|
|
||
| - name: Check PR body | ||
| id: check | ||
| env: | ||
| PR_BODY: ${{ github.event.pull_request.body }} | ||
| run: | | ||
| message="" | ||
| if [ -z "$PR_BODY" ]; then | ||
| message="PR description is empty. Please add a summary of the changes." | ||
| echo "::warning::PR description is empty." | ||
| else | ||
| cleaned=$(echo "$PR_BODY" | sed 's/<!--.*-->//g' | tr -d '[:space:]') | ||
| if [ -z "$cleaned" ]; then | ||
| message="PR description contains only template comments. Please fill in the details." | ||
| echo "::warning::PR description contains only template comments." | ||
| fi | ||
| fi | ||
| echo "message=$message" >> "$GITHUB_OUTPUT" | ||
|
|
||
| - name: Post or clear comment | ||
| if: env.IS_FORK == 'false' | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| run: | | ||
| if [ -n "${{ steps.check.outputs.message }}" ]; then | ||
| printf '%s\n' \ | ||
| "## PR Description (advisory)" \ | ||
| "" \ | ||
| "This is **not blocking**, but your PR description appears to be empty or incomplete." \ | ||
| "" \ | ||
| "${{ steps.check.outputs.message }}" \ | ||
| "" \ | ||
| "A good PR description helps reviewers understand what changed and why." \ | ||
| "" \ | ||
| "---" \ | ||
| "*This comment will be automatically removed once the issue is resolved.*" \ | ||
| | python3 Tools/ci/pr_comment.py --marker pr-body --pr "$PR_NUMBER" --result warn | ||
| else | ||
| python3 Tools/ci/pr_comment.py --marker pr-body --pr "$PR_NUMBER" --result pass | ||
| fi | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,44 +1,150 @@ | ||
| # Contributing to PX4 Firmware | ||
| # Contributing to PX4-Autopilot | ||
|
|
||
| We follow the [Github flow](https://guides.github.com/introduction/flow/) development model. | ||
| We follow the [GitHub flow](https://guides.github.com/introduction/flow/) development model. | ||
|
|
||
| ### Fork the project, then clone your repo | ||
| ## Fork the project, then clone your repo | ||
|
|
||
| First [fork and clone](https://help.github.com/articles/fork-a-repo) the project project. | ||
| First [fork and clone](https://help.github.com/articles/fork-a-repo) the project. | ||
|
|
||
| ### Create a feature branch | ||
| ## Create a feature branch | ||
|
|
||
| *Always* branch off main for new features. | ||
| Always branch off `main` for new features. | ||
|
|
||
| ``` | ||
| git checkout -b mydescriptivebranchname | ||
| ``` | ||
|
|
||
| ### Edit and build the code | ||
| ## Edit and build the code | ||
|
|
||
| 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. | ||
|
|
||
| ### Commit your changes | ||
| ## Commit message convention | ||
|
|
||
| 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) | ||
| PX4 uses [conventional commits](https://www.conventionalcommits.org/) for all commit messages and PR titles. | ||
|
|
||
| **Example:** | ||
| ### Format | ||
|
|
||
| ``` | ||
| Change how the attitude controller works | ||
| type(scope): short description of the change | ||
| ``` | ||
|
|
||
| | Part | Rule | | ||
| |------|------| | ||
| | **type** | Category of change (see types table below) | | ||
| | **scope** | The module, driver, board, or area of PX4 affected | | ||
| | **`!`** (optional) | Append before `:` to mark a breaking change | | ||
| | **description** | What the change does, at least 5 characters, written in imperative form | | ||
|
|
||
| ### Types | ||
|
|
||
| | Type | Description | | ||
| |------|-------------| | ||
| | `feat` | A new feature | | ||
| | `fix` | A bug fix | | ||
| | `docs` | Documentation only changes | | ||
| | `style` | Formatting, whitespace, no code change | | ||
| | `refactor` | Code change that neither fixes a bug nor adds a feature | | ||
| | `perf` | Performance improvement | | ||
| | `test` | Adding or correcting tests | | ||
| | `build` | Build system or external dependencies | | ||
| | `ci` | CI configuration files and scripts | | ||
| | `chore` | Other changes that don't modify src or test files | | ||
| | `revert` | Reverts a previous commit | | ||
|
|
||
| ### Scopes | ||
|
|
||
| The scope identifies which part of PX4 is affected. Common scopes: | ||
|
|
||
| | Scope | Area | | ||
| |-------|------| | ||
| | `ekf2` | Extended Kalman Filter (state estimation) | | ||
| | `mavlink` | MAVLink messaging protocol | | ||
| | `commander` | Commander and mode management | | ||
| | `navigator` | Mission, RTL, Land, and other navigation modes | | ||
| | `sensors` | Sensor drivers and processing | | ||
| | `drivers` | Hardware drivers | | ||
| | `boards/px4_fmu-v6x` | Board-specific changes (use the board name) | | ||
| | `mc_att_control` | Multicopter attitude control | | ||
| | `mc_pos_control` | Multicopter position control | | ||
| | `fw_att_control` | Fixed-wing attitude control | | ||
| | `vtol` | VTOL-specific logic | | ||
| | `actuators` | Mixer and actuator output | | ||
| | `battery` | Battery monitoring and estimation | | ||
| | `logger` | On-board logging | | ||
| | `param` | Parameter system | | ||
| | `simulation` | SITL, Gazebo, SIH | | ||
| | `ci` | Continuous integration and workflows | | ||
| | `docs` | Documentation | | ||
| | `build` | CMake, toolchain, build system | | ||
| | `uorb` | Inter-module messaging | | ||
|
|
||
| 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`. | ||
|
|
||
| ### Breaking changes | ||
|
|
||
| Append `!` before the colon to indicate a breaking change: | ||
|
|
||
| ``` | ||
| feat(ekf2)!: remove deprecated height fusion API | ||
| ``` | ||
|
|
||
| ### Good commit messages | ||
|
|
||
| ``` | ||
| feat(ekf2): add height fusion timeout | ||
| fix(mavlink): correct BATTERY_STATUS_V2 parsing | ||
| refactor(navigator): simplify RTL altitude logic | ||
| ci(workflows): migrate to reusable workflows | ||
| docs(ekf2): update tuning guide | ||
| feat(boards/px4_fmu-v6x)!: remove deprecated driver API | ||
| perf(mc_rate_control): reduce loop latency | ||
| ``` | ||
|
|
||
| ### Commits to avoid | ||
|
|
||
| These will be flagged by CI and should be squashed or reworded before merging: | ||
|
|
||
| ``` | ||
| fix # too vague, no type or scope | ||
| update # too vague, no type or scope | ||
| ekf2: fix something # missing type prefix | ||
| apply suggestions from code review # squash into parent commit | ||
| do make format # squash into parent commit | ||
| WIP: trying something # not ready for main | ||
| oops # not descriptive | ||
| ``` | ||
|
|
||
| ### PR titles | ||
|
|
||
| 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. | ||
|
|
||
| ### Merge policy | ||
|
|
||
| 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`. | ||
|
|
||
| ### Cleaning up commits | ||
|
|
||
| If CI flags your commit messages, you can fix them with an interactive rebase: | ||
|
|
||
| - Fixes rate feed forward | ||
| - Allows a local body rate override | ||
| ```bash | ||
| # Squash all commits into one: | ||
| git rebase -i HEAD~N # replace N with the number of commits | ||
| # mark all commits except the first as 'squash' or 'fixup' | ||
| # reword the remaining commit to follow the format | ||
| git push --force-with-lease | ||
|
|
||
| Fixes issue #123 | ||
| # Or reword specific commits: | ||
| git rebase -i HEAD~N | ||
| # mark the bad commits as 'reword' | ||
| git push --force-with-lease | ||
| ``` | ||
|
|
||
| ### Test your changes | ||
| ## Test your changes | ||
|
|
||
| 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. | ||
| 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. | ||
|
|
||
| ### Push your changes | ||
| ## Push your changes | ||
|
|
||
| Push changes to your repo and send a [pull request](https://github.com/PX4/Firmware/compare/). | ||
| Push changes to your repo and send a [pull request](https://github.com/PX4/PX4-Autopilot/compare/). | ||
|
|
||
| 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. |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.