From 8839e4c13524ee894ecb5d3717e66d7ef88d8ac6 Mon Sep 17 00:00:00 2001 From: Justin Wood Date: Fri, 22 Oct 2021 17:27:05 -0700 Subject: [PATCH] Adding fast path for review guidelines (#10867) * 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 --- .github/.wordlist.txt | 7 ++-- .pullapprove.yml | 81 +++++++++++++++++++++++-------------------- CONTRIBUTING.md | 76 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+), 40 deletions(-) diff --git a/.github/.wordlist.txt b/.github/.wordlist.txt index 7e306fb9cba184..697b2469805f5c 100644 --- a/.github/.wordlist.txt +++ b/.github/.wordlist.txt @@ -28,8 +28,8 @@ AE aef algs alloc -Ameba ameba +Ameba amebad amebaiot AnnounceOTAProvider @@ -131,6 +131,7 @@ cgroup changeset characterised CharString +checkmark ChipBLEMgr CHIPCirqueTest CHIPCryptoPALHsm @@ -324,6 +325,7 @@ EndpointId endpointName eno entrypoint +enum env esd ESPPORT @@ -795,9 +797,9 @@ ScriptBinding SDC SDHC SDK +SDK's sdkconfig SDKs -SDK's SDKTARGETSYSROOT sdl segger @@ -875,6 +877,7 @@ TCP teardown Telink TemperatureMeasurement +testability TestArray TestCluster TestEmptyString diff --git a/.pullapprove.yml b/.pullapprove.yml index 027b944aa496bb..8c7a7d14db8ed5 100644 --- a/.pullapprove.yml +++ b/.pullapprove.yml @@ -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 @@ -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: @@ -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: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 99e21627ac9422..a71219f560671b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -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.