mirror of
https://gitee.com/mirrors_PX4/PX4-Autopilot.git
synced 2026-04-14 10:07:39 +08:00
b142342c3a
* ci(claude): add review-pr skill for domain-aware PR reviews Add a Claude Code skill that reviews pull requests with checks tailored to the domains touched (estimation, control, drivers, simulation, system, CI/build, messages, board additions). Built from analysis of 800+ PR reviews across 8 PX4 maintainers. Includes merge strategy recommendation, interactive dialog for submitting reviews, and human-sounding PR comment formatting. Signed-off-by: Ramon Roche <mrpollo@gmail.com> * ci(copilot): add domain-scoped review instructions for GitHub Copilot Add .github/instructions/ files that give GitHub Copilot PR reviews the same domain-aware context as the Claude Code review-pr skill. Each file is scoped via applyTo to the relevant source paths: core review, estimation, control, drivers/CAN, simulation, system, CI/build, messages/protocol, and board additions. Signed-off-by: Ramon Roche <mrpollo@gmail.com> * fix(claude): address Copilot review feedback - Fix step reference in review-pr skill (step 8 -> step 9) - Capitalize CMake consistently in skill and Copilot instructions Signed-off-by: Ramon Roche <mrpollo@gmail.com> --------- Signed-off-by: Ramon Roche <mrpollo@gmail.com>
33 lines
2.1 KiB
Markdown
33 lines
2.1 KiB
Markdown
---
|
|
applyTo: "src/**,boards/**,platforms/**,msg/**,cmake/**,Makefile,CMakeLists.txt,Tools/**,.github/**"
|
|
---
|
|
|
|
# PX4 Code Review Guidelines
|
|
|
|
## Conventions
|
|
|
|
- PR titles must follow conventional commits: `type(scope): description` (see CONTRIBUTING.md)
|
|
- Types: `feat`, `fix`, `docs`, `style`, `refactor`, `perf`, `test`, `build`, `ci`, `chore`, `revert`
|
|
- Scope should match the primary area of changed files
|
|
- Append `!` before the colon for breaking changes
|
|
- Both squash merge and rebase merge are enabled; merge commits are disabled
|
|
- Commits should be atomic and independently revertable
|
|
- WIP or review-response commits should be squashed before merge
|
|
|
|
## Core Checks (always apply)
|
|
|
|
- **Correctness**: logic errors, off-by-ones, unhandled edge cases
|
|
- **Type safety**: int16 overflow, float/double promotion, unsigned subtraction, use `uint64_t` for absolute time
|
|
- **Initialization**: uninitialized variables, missing default construction
|
|
- **Buffer safety**: unchecked array access, stack allocation of large buffers, snprintf bounds
|
|
- **Magic numbers**: every numeric literal needs a named constant or justification
|
|
- **Framework reuse**: use PX4_ERR/WARN/INFO, existing libraries (AlphaFilter, SlewRate, RateControl), MAVLink constants from the library
|
|
- **Naming**: accurate, no unjustified abbreviations, current terminology (GPS -> GNSS for new code)
|
|
- **Unnecessary complexity**: can code be removed instead of added? Is there a simpler pattern?
|
|
- **Test coverage**: new features should include unit or integration tests; bug fixes should include regression tests where practical
|
|
- **Formatting**: `make format` / `make check_format` (astyle) for C/C++ files; `clang-tidy` clean
|
|
- **Coding style**: C/C++ must follow the PX4 coding style (https://docs.px4.io/main/en/contribute/code.html)
|
|
- **Necessity**: challenge every addition. Is this actually needed or just copied?
|
|
- **Architecture fit**: does the code live in the module that naturally owns the data? No unnecessary cross-module dependencies
|
|
- **Ecosystem impact**: consider QGC users, log analysis tools, and third-party integrations
|