Skip to content

Commit

Permalink
Adding fast path for review guidelines (project-chip#10867)
Browse files Browse the repository at this point in the history
* Update .pullapprove.yml

* Updating for fast path rules

* Typo

* Restyled by prettier-markdown

* Updating word list

* Updating for fast path rules

* Typo

* Restyled by prettier-markdown

* Updating word list

* Restyled by whitespace

Co-authored-by: Restyled.io <commits@restyled.io>
  • Loading branch information
woody-apple and restyled-commits authored Oct 23, 2021
1 parent 8a166dd commit 8839e4c
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 40 deletions.
7 changes: 5 additions & 2 deletions .github/.wordlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ AE
aef
algs
alloc
Ameba
ameba
Ameba
amebad
amebaiot
AnnounceOTAProvider
Expand Down Expand Up @@ -131,6 +131,7 @@ cgroup
changeset
characterised
CharString
checkmark
ChipBLEMgr
CHIPCirqueTest
CHIPCryptoPALHsm
Expand Down Expand Up @@ -324,6 +325,7 @@ EndpointId
endpointName
eno
entrypoint
enum
env
esd
ESPPORT
Expand Down Expand Up @@ -795,9 +797,9 @@ ScriptBinding
SDC
SDHC
SDK
SDK's
sdkconfig
SDKs
SDK's
SDKTARGETSYSROOT
sdl
segger
Expand Down Expand Up @@ -875,6 +877,7 @@ TCP
teardown
Telink
TemperatureMeasurement
testability
TestArray
TestCluster
TestEmptyString
Expand Down
81 changes: 43 additions & 38 deletions .pullapprove.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,46 +4,51 @@ version: 3
github_api_version: "shadow-cat-preview"

############################################################
# Conditions
# Overrides
############################################################

pullapprove_conditions:
############################################################
# License Checks
############################################################
- condition: "'*license/cla*' in statuses.successful"
unmet_status: "pending"
explanation: "CLA must be agreed to by all contributors"
overrides:
- if: "'hotfix' in labels"
status: success
explanation: "Hotfix label added, bypassing reviews"

############################################################
# Draft PRs
############################################################
- condition: "'WIP' not in title"
unmet_status: "pending"
explanation: "Work in progress"
- if: "draft or 'WIP' in title"
status: pending
explanation: "PR is draft, pending review"
- if: "draft"
status: pending
explanation: "PR is draft, pending review"

- condition: "not draft"
unmet_status: "pending"
explanation: "Work in progress"
############################################################
# License Checks
############################################################
- if: "'*license/cla*' not in statuses.successful"
status: pending
explanation: "CLA must be agreed to by all contributors"

############################################################
# Conditions to Skip Review
############################################################
- condition: "base.ref == 'master'"
unmet_status: "success"
- if: "base.ref != 'master'"
status: success
explanation: "Review not required unless merging to master"

############################################################
# Bypass reviews
# Required status checks
############################################################
- "'hotfix' not in labels"
- if: "'*restyle*' not in statuses.successful"
status: failure
explanation: "Style must be inline before reviewing can be complete"

############################################################
# Required status checks
# Fast tracking
############################################################
- condition: "'*restyle*' in statuses.successful"
unmet_status: "failure"
explanation: "Style must be inline before reviewing can be complete"
- if: "'fast track' in labels"
status: success
explanation: "PR has been fast tracked, bypassing reviews"

############################################################
# Notifications
Expand Down Expand Up @@ -91,14 +96,14 @@ groups:
teams: [reviewers-comcast]
reviews:
request: 10
# shared-reviewers-espressif:
# type: optional
# conditions:
# - files.include('*')
# reviewers:
# teams: [reviewers-espressif]
# reviews:
# request: 10
# shared-reviewers-espressif:
# type: optional
# conditions:
# - files.include('*')
# reviewers:
# teams: [reviewers-espressif]
# reviews:
# request: 10
shared-reviewers-google:
type: optional
conditions:
Expand All @@ -107,14 +112,14 @@ groups:
teams: [reviewers-google]
reviews:
request: 10
# shared-reviewers-lg:
# type: optional
# conditions:
# - files.include('*')
# reviewers:
# teams: [reviewers-lg]
# reviews:
# request: 10
# shared-reviewers-lg:
# type: optional
# conditions:
# - files.include('*')
# reviewers:
# teams: [reviewers-lg]
# reviews:
# request: 10
shared-reviewers-nordic:
type: optional
conditions:
Expand Down
76 changes: 76 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,3 +192,79 @@ into master
Documentation undergoes the same review process as code See the
[Documentation Style Guide](https://github.com/project-chip/connectedhomeip/blob/master/docs/STYLE_GUIDE.md)
for more information on how to author and format documentation for contribution.

## Merge Processes

Merges require at least 3 approvals from unique require-reviewers lists, and all
CI tests passing.

### Shorter Reviews

Development Lead & Vice Leads can merge a change with fewer then the required
approvals have been submitted.

A separate "fast track" label will be created that will only require a single
checkbox to be set, this label shall only be set by the Development Lead, and/or
Vice Lead (unless they’re both unavailable, in which case a replacement can be
temporarily appointed)

"Day" here means "business day" (i.e. PRs on friday do not get fast-tracked
faster).

### Fast track types

### Trivial changes

Small changes or changes that do not affect the main functionality of the code
can be fast tracked immediately. Examples:

- Adding/removing documentation (.md files)
- Adding tests (may include small reorganization/method adding/changes to
enable testability):
- certification tests
- stability tests
- integration tests
- functional tests
- Test scripts
- Additional tests following a pattern (e.g. YAML tests)
- Adding/updating/fixing tooling to aid in development
- Re-running code generation
- Code readability refactors:
- renaming enum/classes/structure members
- moving constant header location
- Obviously trivial build rule changes (e.g. adding missing files to build
rules)
- Changing comments
- Adding/removing includes (include what you need and only what you need
rules)
- Pulling new third-party repo files
- Platform vendors/maintainers adding platform features/logic/bug fixes to
their own platforms
- Most changes to existing docker files (pulling new versions, reorganizing)
- Most changes to new dockerfile version in workflows

#### Fast track changes

Larger functionality changes are allowed to be fast tracked with these
requirements/restrictions:

- Require at least 1 day to have passed since the creation of the PR
- Require at least 1 checkmark from someone familiar with the code or problem
space
- This requirement shall be dropped after a PR is 3 days old with stale or
no feedback.
- Code is sufficiently covered by automated tests (or impossible to
automatically test with a very solid reason for this - e.g. changes to BLE
parameters cannot be automatically tested, but should have been manually
verified)

Fast tracking these changes will involve resolving any obviously 'resolved'
comments (judgment call here: were they replied to or addressed) and merging the
change.

Any "request for changes" marker will always be respected unless obviously
resolved (i.e. author marked "requesting changes because of X and X was done in
the PR")

- This requirement shall be dropped after a PR is 3 days old with stale or no
feedback.

0 comments on commit 8839e4c

Please sign in to comment.