addressing pr review comments

This commit intentionally uses a non-conventional commit message to
demonstrate how the new CI checks surface formatting issues. When this
lands on the PR, expect both the PR title check and the commit message
check to flag it with helpful comments showing contributors exactly
what needs to change.

Once everyone has seen the failure in action, this commit should be
squashed into the main migration commit and reworded to follow the
conventional commits format.

Signed-off-by: Ramon Roche <mrpollo@gmail.com>

Author: mrpollo

.github/workflows/commit_checks.yml

@@ -7,6 +7,7 @@ on:
 permissions:
   contents: read
   pull-requests: write
+  issues: write
 
 concurrency:
   group: ${{ github.workflow }}-${{ github.ref }}
@@ -30,7 +31,7 @@ jobs:
       - name: Check PR title
         id: check
         run: |
-          python3 Tools/ci/check_pr_title.py "${{ github.event.pull_request.title }}" --markdown > comment.md 2>/dev/null && rc=0 || rc=$?
+          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
@@ -45,17 +46,10 @@ jobs:
           fi
 
       - name: Result
-        if: always()
+        if: steps.check.outputs.exit_code != '0'
         run: |
-          if [ "${{ steps.check.outputs.exit_code }}" != "0" ]; then
-            # Show the error in the step that fails so it's visible
-            python3 Tools/ci/check_pr_title.py "${{ github.event.pull_request.title }}" 2>&1 || true
-            echo ""
-            echo "::error::PR title does not follow the required format. See output above."
-            exit 1
-          else
-            echo "PR title is valid."
-          fi
+          echo "::error::PR title does not follow conventional commits format. See the PR comment for details."
+          exit 1
 
   commit-messages:
     name: Commit Messages
@@ -72,11 +66,9 @@ jobs:
         env:
           GH_TOKEN: ${{ github.token }}
         run: |
-          commits=$(gh api \
-            "repos/${{ github.repository }}/pulls/${PR_NUMBER}/commits?per_page=100")
-          echo "$commits" | python3 Tools/ci/check_commit_messages.py --markdown > comment.md && rc=0 || rc=$?
-          # Store commits for the result step
-          echo "$commits" > commits.json
+          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
@@ -99,19 +91,10 @@ jobs:
           fi
 
       - name: Result
-        if: always()
+        if: steps.check.outputs.exit_code != '0'
         run: |
-          if [ "${{ steps.check.outputs.exit_code }}" != "0" ]; then
-            # Show the error in the step that fails so it's visible
-            python3 Tools/ci/check_commit_messages.py < commits.json || true
-            echo ""
-            echo "::error::Commit message errors found. See output above."
-            exit 1
-          elif [ "${{ steps.check.outputs.has_warnings }}" == "true" ]; then
-            python3 Tools/ci/check_commit_messages.py < commits.json || true
-          else
-            echo "All commit messages are valid."
-          fi
+          echo "::error::Commit message errors found. See the PR comment for details."
+          exit 1
 
   pr-body:
     name: PR Description

CONTRIBUTING.md

@@ -20,65 +20,94 @@ The [developer guide](https://docs.px4.io/main/en/development/development.html)
 
 ## Commit message convention
 
-PX4 uses the `subsystem: description` format for all commit messages and PR titles.
+PX4 uses [conventional commits](https://www.conventionalcommits.org/) for all commit messages and PR titles.
 
 ### Format
 
 ```
-subsystem: short description of the change
+type(scope): short description of the change
 ```
 
 | Part | Rule |
 |------|------|
-| **subsystem** | The module, driver, board, or area of PX4 that the change affects |
-| **`:`** + space | Required separator |
+| **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 |
 
-### Subsystem examples
+### Types
 
-Common subsystem prefixes used in PX4:
+| 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 |
 
-| Subsystem | Area |
-|-----------|------|
+### 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) |
-| `multicopter` | Multicopter-specific control and estimation |
-| `fixedwing` | Fixed-wing-specific control and estimation |
+| `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 |
+| `ci` | Continuous integration and workflows |
 | `docs` | Documentation |
 | `build` | CMake, toolchain, build system |
-| `uORB` | Inter-module messaging |
+| `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`.
 
-For changes spanning multiple subsystems, use the primary one affected.
+### Breaking changes
+
+Append `!` before the colon to indicate a breaking change:
+
+```
+feat(ekf2)!: remove deprecated height fusion API
+```
 
 ### Good commit messages
 
 ```
-ekf2: fix height fusion timeout
-mavlink: add BATTERY_STATUS_V2 support
-boards/px4_fmu-v6x: enable UAVCAN
-navigator: fix RTL altitude calculation
-CI: migrate to reusable workflows
-docs: update EKF tuning guide
+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 subsystem
-update                                 # too vague, no subsystem
+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
@@ -87,7 +116,11 @@ oops                                   # not descriptive
 
 ### PR titles
 
-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.
+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