* Trim copilot PR review instructions * Remove unactionable and subjective rules * Limit review to keyboards only * Defer advance code review to collaborators * Improve non-pristine default keymap message * Clarify info.json rules for PR review * Narrow applyTo scope to keyboards/**
11 KiB
| applyTo | excludeAgent | |
|---|---|---|
| keyboards/** |
|
GitHub Copilot Instructions for QMK Pull Request Review
This document provides automated review guidance based on the QMK PR Checklist and it is intended only for use by GitHub Copilot code-review agent during pull request reviews.
Scope: This review applies only to changes within the
keyboards/folder. Changes outsidekeyboards/(e.g., core code, quantum, platforms, drivers) must be flagged and deferred to a QMK Collaborator — do not review them.
General PR Requirements
Branch and Submission Standards
- Source Branch Policy: Verify PR is NOT submitted from submitter's own
masterbranch- Flag if submitter is using their own
masterbranch as source - Suggest using feature branches instead for cleaner fork management
- Flag if submitter is using their own
- Target Branch Policy:
- New keyboard additions →
masterbranch (new folders underkeyboards/) - All other changes →
developbranch:- Keyboard updates, refactors, or moves
- Core code changes
- Data-driven configuration migrations
- Any modifications to existing keyboards
- New keyboard additions →
- PR Scope: PRs should contain the smallest set of modifications for a single change
- Flag PRs that modify multiple keyboards simultaneously
- Suggest splitting large PRs into focused, incremental changes
- Merge Conflicts: Check for unresolved merge conflicts
File Naming and Structure
- Lowercase Requirement: All new directories and filenames must be lowercase
- Exception: Upstream sources with original uppercase (LUFA, ChibiOS)
- Exception: Core files with valid justification
- Reject: Board designer preference for uppercase is NOT valid justification
License Headers
- Required: Valid license headers on all
*.cand*.hfiles - Recommended: GPL2/GPL3 for consistency
- Format: Check for proper GPL2+ header or SPDX identifier
// Copyright 2024 Your Name (@yourgithub) // SPDX-License-Identifier: GPL-2.0-or-later - Exception: Simple assignment-only
rules.mkfiles don't need headers - Flag: Missing or ambiguous license headers (blocks merge)
Keymap PR Reviews
Scope: These rules apply to files within keyboards/*/keymaps/* subdirectories.
Note on Non-Default Keymaps
- Any keymap folder named something other than
default(e.g.,default_clueboard,via) is a non-default keymap - Do not review the content of non-default keymaps — flag their presence and note that a QMK Collaborator must review them
Keymap Code Standards
- Includes:
#include QMK_KEYBOARD_Hpreferred over specific board files - Enums: Suggest using layer enums instead of
#defines for layers (soft suggestion only) - Custom Keycodes: First entry must be
QK_USER - VIA: Keymaps should NOT enable VIA
- VIA keymaps belong in VIA QMK Userspace
Keyboard PR Reviews
Scope: These rules apply to keyboard-level files in keyboards/* directories, excluding files within the keymaps/ subdirectories. This includes:
info.jsonorkeyboard.json(keyboard root or variant level)readme.md(keyboard level)rules.mk(keyboard level)config.h(keyboard level, not keymap level)<keyboard>.cand<keyboard>.hfiles- Hardware configuration files (
halconf.h,mcuconf.h,chconf.h)
Branch Targeting
- New Keyboards: Target
masterbranch- New additions to
keyboards/folder submit tomaster
- New additions to
- Keyboard Moves: Must target
developbranch- Check
data/mappings/keyboard_aliases.hjsonis updated for moves
- Check
- Keyboard Updates/Refactors: Must target
developto reduce merge conflicts - Data Driven Migration: Must target
develop
info.json and keyboard.json Requirements
- Schema Validation: Verify that
info.jsonandkeyboard.jsonfiles are valid JSON (no syntax errors, no trailing commas, properly closed brackets) - Mandatory Elements:
urlfield is present and begins withhttp://orhttps://maintainerfield is present and non-emptyusb.vidandusb.pidfields are present and match the0xhexadecimal format (e.g."vid": "0x1234","pid": "0xFEED")usb.device_versionfield is present- Each key entry in every
layoutarray must include a"matrix"property (e.g.{"label": "L01", "matrix": [0, 0], "x": 0, "y": 0}) - Microcontroller and bootloader specified
- Diode direction (if not using direct pins)
- Layout Naming:
- Single layout: Must be named
LAYOUTor matchLAYOUT_*pattern - Multiple layouts: Must include
LAYOUT_allplus at least one alternate name
- Single layout: Must be named
readme.md Requirements
- Template: Must follow official template
- Flash Command: Present with
:flashat end - Hardware Link: Valid availability link (unless handwired)
- Private groupbuys acceptable
- Open-source should link to files
- Reset Instructions: Bootloader reset instructions must be present (non-empty)
- Images: A keyboard image is preferred but not mandatory
- If present, images must not be hosted in the repository
- If an imgur URL is used, it must link directly to the image (i.e.
https://i.imgur.com/<id>.<ext>), not the preview page (i.e. nothttps://imgur.com/<id>)
rules.mk Standards
- Removed Items:
- Size comments like
(-/+size)
- Size comments like
config.h Standards (Keyboard Level)
- Prohibited:
#define DESCRIPTION- Comment blocks explaining features
- Commented-out unused defines
#include "config_common.h"
- No Vial: Vial-related files/changes not accepted
Keyboard Implementation Files
<keyboard>.c
- Advanced Code Flag: ⚠️ If a
<keyboard>.cfile is present in the PR, flag it as possible advanced code requiring QMK Collaborator review
<keyboard>.h
- Include:
#include "quantum.h"at top - Layout Macros: If
LAYOUTmacros are present, suggest moving them toinfo.jsonorkeyboard.json
Default Keymap Standards
Scope: These rules specifically apply to files within keyboards/*/keymaps/default/ directories.
- Pristine Requirement: Bare minimum clean slate
- ⚠️ If
keymap.ccontains anything beyond thekeymaps[]array, or if thedefaultkeymap folder contains additional.cor.hfiles, flag this as possible advanced code requiring QMK Collaborator review
- ⚠️ If
- Removed Examples: Delete
QMKBEST/QMKURLmacros - Tri Layer: Use Tri Layer feature instead of manual
layer_on/off()+update_tri_layer() - Encoder Map: Use encoder map feature,
encoder_update_user()may not be present - No VIA: Default keymap should not enable VIA
Prohibited Files
- No VIA JSON: Belongs in VIA Keyboard Repo
- No KLE JSON: Not used within QMK
- Single Keyboard Per PR: A PR must not modify more than one
keyboards/<folder>/directory- Flag PRs that touch multiple top-level keyboard folders (e.g., both
keyboards/mykeyboard1/andkeyboards/mykeyboard2/) — each keyboard must be a separate PR
- Flag PRs that touch multiple top-level keyboard folders (e.g., both
- No Cross-Keyboard Sources: Don't include files from other keyboard vendors
- Exception: Core files (e.g.,
drivers/sensors/pmw3360.c) - Use of vendor-specific code (e.g.,
wilba_tech/wt_main.c) only when keyboard exists in the same enclosing vendor folder (e.g. awilba_techkeyboard)
- Exception: Core files (e.g.,
Review Response Templates
For source master branch usage:
⚠️ This PR appears to be submitted from your own `master` branch. For future PRs, we recommend using feature branches instead of committing to your `master`. This makes it easier to keep your fork updated and manage multiple PRs.
See: [Best Practices: Your Fork's Master](https://docs.qmk.fm/newbs_git_using_your_master_branch)
For incorrect target branch:
❌ This PR targets the wrong branch:
- **New keyboard additions** should target `master`
- **Keyboard updates/refactors/moves** should target `develop`
Please change the target branch accordingly.
For missing license headers:
❌ Missing GPL-compatible license headers on the following files:
- [list files]
Please add GPL2+ headers (GPL2/GPL3 recommended). Example:
\`\`\`c
// Copyright 2024 Your Name (@yourgithub)
// SPDX-License-Identifier: GPL-2.0-or-later
\`\`\`
For non-lowercase filenames:
❌ The following files/directories must be lowercase:
- [list files]
Exception: Only valid if from upstream sources (LUFA, ChibiOS) or justified by core consistency.
For config.h violations:
⚠️ Found prohibited config.h elements:
- [list specific issues: `#define DESCRIPTION`, `#include "config_common.h"`, comment blocks, commented-out defines]
Please remove these and refer to [Data Driven Configuration](https://docs.qmk.fm/data_driven_config).
For info.json or keyboard.json issues:
⚠️ info.json or keyboard.json needs attention:
- [list missing mandatory fields]
- Please run: \`qmk format-json -i path/to/info.json\` (or keyboard.json)
- Validate with: \`qmk lint -kb <keyboard_name>\`
For non-pristine default keymap:
⚠️ Default keymap requires collaborator review:
The default `keymap.c` contains code beyond the `keymaps[]` array. The following was detected:
- [describe: extra functions/definitions in keymap.c, or additional .c/.h files in the default keymap folder]
A QMK Collaborator must review this before merging.
For non-default keymaps:
⚠️ Non-default keymap detected: `keyboards/[path]/keymaps/[name]/`
This keymap requires a QMK Collaborator to review its content. No automated review of its code was performed.
For <keyboard>.c detected:
⚠️ `<keyboard>.c` file detected: `keyboards/[path]/[keyboard].c`
This file may contain advanced hardware configuration. A QMK Collaborator must review this before merging.
Constraints and Conduct
- Scope: Only review changes within the
keyboards/folder. Flag any changes outsidekeyboards/and defer to a QMK Collaborator without reviewing them. - Focus reviews on objective checklist items that can be automatically verified
- Flag definite violations with ❌
- Suggest improvements for recommendations with ⚠️
- Provide specific file/line references when flagging issues
- Link to relevant QMK documentation for each issue
- Prioritize blocking issues (license, merge conflicts, branch policy)
- Be constructive: Suggest fixes, not just problems
- Do not review non-default keymaps — flag their presence and defer to a QMK Collaborator
- Do not review
<keyboard>.ccontent in detail — flag its presence and defer to a QMK Collaborator
This is meant as a first-pass review to catch common issues before human review. Complex architectural decisions, code quality, and subjective assessments still require human QMK Collaborator review.