diff options
Diffstat (limited to 'docs/pr_checklist.md')
| -rw-r--r-- | docs/pr_checklist.md | 24 |
1 files changed, 18 insertions, 6 deletions
diff --git a/docs/pr_checklist.md b/docs/pr_checklist.md index f55517dfa2..6d74e246a8 100644 --- a/docs/pr_checklist.md +++ b/docs/pr_checklist.md @@ -46,9 +46,15 @@ Closed PRs (for inspiration, previous sets of review comments will help you elim https://github.com/qmk/qmk_firmware/pulls?q=is%3Apr+is%3Aclosed+label%3Akeyboard - `info.json` - - valid URL - - valid maintainer - - displays correctly in Configurator (press Ctrl+Shift+I to preview local file, turn on fast input to verify ordering) + - With the move to [data driven](https://docs.qmk.fm/#/data_driven_config) keyboard configuration, we encourage contributors to utilise as many features as possible of the info.json [schema](https://github.com/qmk/qmk_firmware/blob/master/data/schemas/keyboard.jsonschema). + - the mandatory elements for a minimally complete `info.json` at present are: + - valid URL + - valid maintainer + - valid USB VID/PID and device version + - displays correctly in Configurator (press Ctrl+Shift+I to preview local file, turn on fast input to verify ordering) + - `layout` definitions should include matrix positions, so that `LAYOUT` macros can be generated at build time + - should use standard definitions if applicable + - use the Community Layout macro names where they apply (preferred above `LAYOUT`/`LAYOUT_all`) - `readme.md` - standard template should be present -- [link to template](https://github.com/qmk/qmk_firmware/blob/master/data/templates/keyboard/readme.md) - flash command is present, and has `:flash` at end @@ -57,12 +63,16 @@ https://github.com/qmk/qmk_firmware/pulls?q=is%3Apr+is%3Aclosed+label%3Akeyboard - a picture about the keyboard and preferably about the PCB, too - images are not to be placed in the `qmk_firmware` repository - images should be uploaded to an external image hosting service, such as [imgur](https://imgur.com/). + - if imgur is used, images should be resized appropriately: append "h" to the image url i.e. `https://i.imgur.com/vqgE7Ok.jpg` becomes `https://i.imgur.com/vqgE7Okh.jpg` - `rules.mk` - removed `MIDI_ENABLE`, `FAUXCLICKY_ENABLE` and `HD44780_ENABLE` - modified `# Enable Bluetooth with the Adafruit EZ-Key HID` -> `# Enable Bluetooth` - no `(-/+size)` comments related to enabling features - remove the list of alternate bootloaders if one has been specified - no re-definitions of the default MCU parameters if same value, when compared to the equivalent MCU in [mcu_selection.mk](https://github.com/qmk/qmk_firmware/blob/master/builddefs/mcu_selection.mk) + - no "keymap only" features enabled + - `COMBO_ENABLE` + - `ENCODER_MAP_ENABLE` - keyboard `config.h` - don't repeat `MANUFACTURER` in the `PRODUCT` value - no `#define DESCRIPTION` @@ -75,14 +85,15 @@ https://github.com/qmk/qmk_firmware/pulls?q=is%3Apr+is%3Aclosed+label%3Akeyboard - Vial-related files or changes will not be accepted, as they are not used by QMK firmware (no Vial-specific core code has been submitted or merged) - `<keyboard>.c` - empty `xxxx_xxxx_kb()` or other weak-defined default implemented functions removed + - empty `xxxx_xxxx_user()` or other user-level functions are disallowed at the keyboard level and must be moved to keymaps - commented-out functions removed too - `matrix_init_board()` etc. migrated to `keyboard_pre_init_kb()`, see: [keyboard_pre_init*](custom_quantum_functions.md?id=keyboard_pre_init_-function-documentation) - prefer `CUSTOM_MATRIX = lite` if custom matrix used, allows for standard debounce, see [custom matrix 'lite'](custom_matrix.md?id=lite) - prefer LED indicator [Configuration Options](feature_led_indicators.md?id=configuration-options) to custom `led_update_*()` implementations where possible + - Encoder support should not require any keyboard-level code, and associated keymaps should now leverage the [Encoder Map](feature_encoders.md?id=encoder-map) feature instead. - `<keyboard>.h` - `#include "quantum.h"` appears at the top - - `LAYOUT` macros should use standard definitions if applicable - - use the Community Layout macro names where they apply (preferred above `LAYOUT`/`LAYOUT_all`) + - `LAYOUT` macros should be moved to `info.json` - keymap `config.h` - no duplication of `rules.mk` or `config.h` from keyboard - `keymaps/default/keymap.c` @@ -101,7 +112,7 @@ https://github.com/qmk/qmk_firmware/pulls?q=is%3Apr+is%3Aclosed+label%3Akeyboard - submitters can have a personal (or bells-and-whistles) keymap showcasing capabilities in the same PR but it shouldn't be embedded in the 'default' keymap - submitters can also have a "manufacturer-matching" keymap that mirrors existing functionality of the commercial product, if porting an existing board - Do not include VIA json files in the PR. These do not belong in the QMK repository as they are not used by QMK firmware -- they belong in the [VIA Keyboard Repo](https://github.com/the-via/keyboards) -- Do not include source files from another keyboard or vendors keyboard folder. Including core files is fine. +- Do not include source files from another keyboard or vendors keyboard folder. Including core files is fine. - For instance, only `wilba_tech` boards using be including `keyboards/wilba_tech/wt_main.c` and `keyboards/wilba_tech/wt_rgb_backlight.c`. But including `drivers/sensors/pmw3360.c` is absolutely fine. - Code that needs to be used by multiple boards is a candidate for core code changes, and should be separated out. @@ -124,6 +135,7 @@ Also, specific to ChibiOS: - for new MCUs, a new "child" keyboard should be added that targets your newly-added MCU, so that builds can be verified - for new hardware support such as display panels, core-side matrix implementations, or other peripherals, an associated keymap should be provided - if an existing keymap exists that can leverage this functionality this may not be required (e.g. a new RGB driver chip, supported by the `rgb` keymap) -- consult with the QMK Collaborators on Discord to determine if there is sufficient overlap already +- any features adding `_kb`/`_user` callbacks must return a `bool`, to allow for user override of keyboard-level callbacks. - other requirements are at the discretion of QMK collaborators - core is a lot more subjective given the breadth of posted changes |