aboutsummaryrefslogtreecommitdiff
path: root/docs
diff options
context:
space:
mode:
authorNick Brassel <nick@tzarc.org>2021-10-06 06:46:36 +1100
committerGitHub <noreply@github.com>2021-10-06 06:46:36 +1100
commit4676a14596f8ad4065acd2e18c892e0ee61ee346 (patch)
tree467e9e4506d70d2610976bf083c02f08c9616cb9 /docs
parent7a49e5d2078e8e1044f11d0d7d2a67e82867024a (diff)
downloadqmk_firmware-4676a14596f8ad4065acd2e18c892e0ee61ee346.tar.gz
qmk_firmware-4676a14596f8ad4065acd2e18c892e0ee61ee346.zip
More PR checklist updates (#14705)
* Wording, clarification. * Apply suggestions from code review Co-authored-by: Mikkel Jeppesen <2756925+Duckle29@users.noreply.github.com> Co-authored-by: Mikkel Jeppesen <2756925+Duckle29@users.noreply.github.com>
Diffstat (limited to 'docs')
-rw-r--r--docs/pr_checklist.md34
1 files changed, 21 insertions, 13 deletions
diff --git a/docs/pr_checklist.md b/docs/pr_checklist.md
index cac3312e0..817ed22d4 100644
--- a/docs/pr_checklist.md
+++ b/docs/pr_checklist.md
@@ -4,14 +4,14 @@ This is a non-exhaustive checklist of what the QMK Collaborators will be checkin
4 4
5If there are any inconsistencies with these recommendations, you're best off [creating an issue](https://github.com/qmk/qmk_firmware/issues/new) against this document, or getting in touch with a QMK Collaborator on [Discord](https://discord.gg/Uq7gcHh). 5If there are any inconsistencies with these recommendations, you're best off [creating an issue](https://github.com/qmk/qmk_firmware/issues/new) against this document, or getting in touch with a QMK Collaborator on [Discord](https://discord.gg/Uq7gcHh).
6 6
7## General PRs 7## Requirements for all PRs
8 8
9- PR should be submitted using a non-`master` branch on the source repository 9- PR should be submitted using a non-`master` branch on the source repository
10 - this does not mean you target a different branch for your PR, rather that you're not working out of your own master branch 10 - this does not mean you target a different branch for your PR, rather that you're not working out of your own master branch
11 - if submitter _does_ use their own `master` branch, they'll be given a link to the ["how to git"](https://docs.qmk.fm/#/newbs_git_using_your_master_branch) page after merging -- (end of this document will contain the contents of the message) 11 - if submitter _does_ use their own `master` branch, they'll be given a link to the ["how to git"](https://docs.qmk.fm/#/newbs_git_using_your_master_branch) page after merging -- (end of this document will contain the contents of the message)
12- newly-added directories and filenames must be lowercase 12- newly-added directories and filenames must be lowercase
13 - this rule may be relaxed if upstream sources originally had uppercase characters (e.g. ChibiOS, or imported files from other repositories etc.) 13 - this rule may be relaxed if upstream sources originally had uppercase characters (e.g. LUFA, ChibiOS, or imported files from other repositories etc.)
14 - if there is enough justification (i.e. consistency with existing core files etc.) this can be relaxed 14 - if there is valid justification (i.e. consistency with existing core files etc.) this can be relaxed
15 - a board designer naming their keyboard with uppercase letters is not enough justification 15 - a board designer naming their keyboard with uppercase letters is not enough justification
16- valid license headers on all `*.c` and `*.h` source files 16- valid license headers on all `*.c` and `*.h` source files
17 - GPL2/GPL3 recommended for consistency 17 - GPL2/GPL3 recommended for consistency
@@ -21,7 +21,7 @@ If there are any inconsistencies with these recommendations, you're best off [cr
21- QMK Codebase "best practices" followed 21- QMK Codebase "best practices" followed
22 - this is not an exhaustive list, and will likely get amended as time goes by 22 - this is not an exhaustive list, and will likely get amended as time goes by
23 - `#pragma once` instead of `#ifndef` include guards in header files 23 - `#pragma once` instead of `#ifndef` include guards in header files
24 - no "old-school" GPIO/I2C/SPI functions used -- must use QMK abstractions unless justifiable (and laziness is not valid justification) 24 - no "old-school" or other low-level GPIO/I2C/SPI functions may be used -- must use QMK abstractions unless justifiable (and laziness is not valid justification)
25 - timing abstractions should be followed too: 25 - timing abstractions should be followed too:
26 - `wait_ms()` instead of `_delay_ms()` (remove `#include <util/delay.h>` too) 26 - `wait_ms()` instead of `_delay_ms()` (remove `#include <util/delay.h>` too)
27 - `timer_read()` and `timer_read32()` etc. -- see [timer.h](https://github.com/qmk/qmk_firmware/blob/master/tmk_core/common/timer.h) for the timing APIs 27 - `timer_read()` and `timer_read32()` etc. -- see [timer.h](https://github.com/qmk/qmk_firmware/blob/master/tmk_core/common/timer.h) for the timing APIs
@@ -30,7 +30,7 @@ If there are any inconsistencies with these recommendations, you're best off [cr
30 - discuss it with QMK Collaborators on Discord 30 - discuss it with QMK Collaborators on Discord
31 - refactor it as a separate core change 31 - refactor it as a separate core change
32 - remove your specific copy in your board 32 - remove your specific copy in your board
33- rebase and fix all merge conflicts before opening the PR (in case you need help or advice, reach out to QMK Collaborators on Discord) 33- fix all merge conflicts before opening the PR (in case you need help or advice, reach out to QMK Collaborators on Discord)
34 34
35## Keymap PRs 35## Keymap PRs
36 36
@@ -50,11 +50,13 @@ https://github.com/qmk/qmk_firmware/pulls?q=is%3Apr+is%3Aclosed+label%3Akeyboard
50 - valid maintainer 50 - valid maintainer
51 - displays correctly in Configurator (press Ctrl+Shift+I to preview local file, turn on fast input to verify ordering) 51 - displays correctly in Configurator (press Ctrl+Shift+I to preview local file, turn on fast input to verify ordering)
52- `readme.md` 52- `readme.md`
53 - standard template should be present 53 - standard template should be present -- [link to template](https://github.com/qmk/qmk_firmware/blob/master/data/templates/avr/readme.md)
54 - flash command has `:flash` at end 54 - flash command is present, and has `:flash` at end
55 - valid hardware availability link (unless handwired) -- private groupbuys are okay, but one-off prototypes will be questioned. If open-source, a link to files should be provided. 55 - valid hardware availability link (unless handwired) -- private groupbuys are okay, but one-off prototypes will be questioned. If open-source, a link to files should be provided.
56 - clear instructions on how to reset the board into bootloader mode 56 - clear instructions on how to reset the board into bootloader mode
57 - a picture about the keyboard and preferably about the PCB, too 57 - a picture about the keyboard and preferably about the PCB, too
58 - images are not to be placed in the `qmk_firmware` repository
59 - images should be uploaded to an external image hosting service, such as [imgur](https://imgur.com/).
58- `rules.mk` 60- `rules.mk`
59 - removed `MIDI_ENABLE`, `FAUXCLICKY_ENABLE` and `HD44780_ENABLE` 61 - removed `MIDI_ENABLE`, `FAUXCLICKY_ENABLE` and `HD44780_ENABLE`
60 - modified `# Enable Bluetooth with the Adafruit EZ-Key HID` -> `# Enable Bluetooth` 62 - modified `# Enable Bluetooth with the Adafruit EZ-Key HID` -> `# Enable Bluetooth`
@@ -71,20 +73,20 @@ https://github.com/qmk/qmk_firmware/pulls?q=is%3Apr+is%3Aclosed+label%3Akeyboard
71 - initialisation code for the matrix and critical devices 73 - initialisation code for the matrix and critical devices
72 - mirroring existing functionality of a commercial board (like custom keycodes and special animations etc.) should be handled through non-`default` keymaps 74 - mirroring existing functionality of a commercial board (like custom keycodes and special animations etc.) should be handled through non-`default` keymaps
73 - 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) 75 - 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)
74- `keyboard.c` 76- `<keyboard>.c`
75 - empty `xxxx_xxxx_kb()` or other weak-defined default implemented functions removed 77 - empty `xxxx_xxxx_kb()` or other weak-defined default implemented functions removed
76 - commented-out functions removed too 78 - commented-out functions removed too
77 - `matrix_init_board()` etc. migrated to `keyboard_pre_init_kb()`, see: [keyboard_pre_init*](https://docs.qmk.fm/#/custom_quantum_functions?id=keyboard_pre_init_-function-documentation) 79 - `matrix_init_board()` etc. migrated to `keyboard_pre_init_kb()`, see: [keyboard_pre_init*](https://docs.qmk.fm/#/custom_quantum_functions?id=keyboard_pre_init_-function-documentation)
78 - prefer `CUSTOM_MATRIX = lite` if custom matrix used, allows for standard debounce, see [custom matrix 'lite'](https://docs.qmk.fm/#/custom_matrix?id=lite) 80 - prefer `CUSTOM_MATRIX = lite` if custom matrix used, allows for standard debounce, see [custom matrix 'lite'](https://docs.qmk.fm/#/custom_matrix?id=lite)
79 - prefer LED indicator [Configuration Options](https://docs.qmk.fm/#/feature_led_indicators?id=configuration-options) to custom `led_update_*()` implementations where possible 81 - prefer LED indicator [Configuration Options](https://docs.qmk.fm/#/feature_led_indicators?id=configuration-options) to custom `led_update_*()` implementations where possible
80- `keyboard.h` 82- `<keyboard>.h`
81 - `#include "quantum.h"` appears at the top 83 - `#include "quantum.h"` appears at the top
82 - `LAYOUT` macros should use standard definitions if applicable 84 - `LAYOUT` macros should use standard definitions if applicable
83 - use the Community Layout macro names where they apply (preferred above `LAYOUT`/`LAYOUT_all`) 85 - use the Community Layout macro names where they apply (preferred above `LAYOUT`/`LAYOUT_all`)
84- keymap `config.h` 86- keymap `config.h`
85 - no duplication of `rules.mk` or `config.h` from keyboard 87 - no duplication of `rules.mk` or `config.h` from keyboard
86- `keymaps/default/keymap.c` 88- `keymaps/default/keymap.c`
87 - `QMKBEST`/`QMKURL` removed (sheesh) 89 - `QMKBEST`/`QMKURL` removed
88 - if using `MO(_LOWER)` and `MO(_RAISE)` keycodes or equivalent, and the keymap has an adjust layer when holding both keys -- if the keymap has no "direct-to-adjust" keycode (such as `MO(_ADJUST)`) then you should prefer to write... 90 - if using `MO(_LOWER)` and `MO(_RAISE)` keycodes or equivalent, and the keymap has an adjust layer when holding both keys -- if the keymap has no "direct-to-adjust" keycode (such as `MO(_ADJUST)`) then you should prefer to write...
89 ``` 91 ```
90 layer_state_t layer_state_set_user(layer_state_t state) { 92 layer_state_t layer_state_set_user(layer_state_t state) {
@@ -100,7 +102,6 @@ https://github.com/qmk/qmk_firmware/pulls?q=is%3Apr+is%3Aclosed+label%3Akeyboard
100- submitters can also have a "manufacturer-matching" keymap that mirrors existing functionality of the commercial product, if porting an existing board 102- submitters can also have a "manufacturer-matching" keymap that mirrors existing functionality of the commercial product, if porting an existing board
101- 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) 103- 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)
102 104
103
104Also, specific to ChibiOS: 105Also, specific to ChibiOS:
105- **strong** preference to using existing ChibiOS board definitions. 106- **strong** preference to using existing ChibiOS board definitions.
106 - a lot of the time, an equivalent Nucleo board can be used with a different flash size or slightly different model in the same family 107 - a lot of the time, an equivalent Nucleo board can be used with a different flash size or slightly different model in the same family
@@ -114,7 +115,7 @@ Also, specific to ChibiOS:
114## Core PRs 115## Core PRs
115 116
116- must now target `develop` branch, which will subsequently be merged back to `master` on the breaking changes timeline 117- must now target `develop` branch, which will subsequently be merged back to `master` on the breaking changes timeline
117- other notes TBD 118- other requirements are at the discretion of QMK collaborators
118 - core is a lot more subjective given the breadth of posted changes 119 - core is a lot more subjective given the breadth of posted changes
119 120
120--- 121---
@@ -136,7 +137,7 @@ Thanks for contributing!
136 137
137## Review Process 138## Review Process
138 139
139In general, we want to see two (or more) approvals that are meaningful (e.g. that have inspected code) before a PR will be considered for merge. These reviews are not limited to collaborators -- any community member willing to put in the time is welcomed (and encouraged). The only difference is that your checkmark won't be green, and that's fine! 140In general, we want to see two (or more) approvals that are meaningful (e.g. that have inspected code) before a PR will be considered for merge. These reviews are not limited to collaborators -- any community member willing to put in the time is welcomed (and encouraged). The only difference is that your checkmark won't be green, and that's fine!
140 141
141Additionally, PR reviews are something that is done in our free time. We are not paid nor compensated for the time we spend reviewing, as it is a labor of love. As such, this means that it can take time for us to get to your Pull Request. Things like family, or life can get in the way of us getting to PRs, and burnout is a serious concern. The QMK firmware repository averages 200 PRs opened and 200 PRs merged every month, so please have patience. 142Additionally, PR reviews are something that is done in our free time. We are not paid nor compensated for the time we spend reviewing, as it is a labor of love. As such, this means that it can take time for us to get to your Pull Request. Things like family, or life can get in the way of us getting to PRs, and burnout is a serious concern. The QMK firmware repository averages 200 PRs opened and 200 PRs merged every month, so please have patience.
142 143
@@ -159,3 +160,10 @@ Additionally, PR reviews are something that is done in our free time. We are not
159 * along with this program. If not, see <http://www.gnu.org/licenses/>. 160 * along with this program. If not, see <http://www.gnu.org/licenses/>.
160 */ 161 */
161``` 162```
163
164Or, optionally, using [SPDX identifier](https://spdx.org/licenses/) instead:
165
166```
167// Copyright 2021 Your Name (@yourgithub)
168// SPDX-License-Identifier: GPL-2.0-or-later
169```