From 61aed9154d247736a0d7707a79dcfa60c72af48f Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Fri, 12 Mar 2021 13:46:04 -0700 Subject: [PATCH 1/5] Add new PR template for master --- pull_request_template.md | 37 ++++++------------------------------- 1 file changed, 6 insertions(+), 31 deletions(-) diff --git a/pull_request_template.md b/pull_request_template.md index c374b58fbd..07ac7d9d5a 100644 --- a/pull_request_template.md +++ b/pull_request_template.md @@ -1,34 +1,9 @@ -This PR resolves #... -Changes in this PR: +# Code-Reviewer Section +The general guidlines can be found [here](https://github.com/apple/foundationdb/wiki/FoundationDB-Commit-Process). -- -- -- +Please check each of the following things and check *all* boxed before accepting a PR. -## General guideline: - -- If this PR is ready to be merged (and all checkboxes below are either ticked or not applicable), make this a regular PR -- If this PR still needs work, please make this a draft PR - - If you wish to get feedback/code-review, please add the label RFC to this PR - -Please verify that all things listed below were considered and check them. If an item doesn't apply to this type of PR (for example a documentation change doesn't need to be performance tested), you should make a ~~strikethrough~~ (markdown syntax: `~~strikethrough~~`). More infos on the guidlines can be found [here](https://github.com/apple/foundationdb/wiki/FoundationDB-Commit-Process). - -### Style - -- [ ] All variable and function names make sense. -- [ ] The code is properly formatted (consider running `git clang-format`). - -### Performance - -- [ ] All CPU-hot paths are well optimized. -- [ ] The proper containers are used (for example `std::vector` vs `VectorRef`). -- [ ] There are no new known `SlowTask` traces. - -### Testing - -- [ ] The code was sufficiently tested in simulation. -- [ ] If there are new parameters or knobs, different values are tested in simulation. -- [ ] `ASSERT`, `ASSERT_WE_THINK`, and `TEST` macros are added in appropriate places. -- [ ] Unit tests were added for new algorithms and data structure that make sense to unit-test -- [ ] If this is a bugfix: there is a test that can easily reproduce the bug. +[ ] The PR has a description. +[ ] The description mentions which forms of testing were done and the testing seems reasonable. +[ ] Every function/class/actor that was touched is reasonably well documented. From 0449ae93e8b75e895c5218635076c01cda8d4c4f Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Fri, 12 Mar 2021 13:47:26 -0700 Subject: [PATCH 2/5] description string --- pull_request_template.md | 1 + 1 file changed, 1 insertion(+) diff --git a/pull_request_template.md b/pull_request_template.md index 07ac7d9d5a..0f5971ffbc 100644 --- a/pull_request_template.md +++ b/pull_request_template.md @@ -1,3 +1,4 @@ +Put description here... # Code-Reviewer Section The general guidlines can be found [here](https://github.com/apple/foundationdb/wiki/FoundationDB-Commit-Process). From 9cb3ae887e5f904c82a33d01dbef43a19f6823d3 Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Fri, 12 Mar 2021 15:49:03 -0700 Subject: [PATCH 3/5] Address review comments --- pull_request_template.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pull_request_template.md b/pull_request_template.md index 0f5971ffbc..3dd8732ef5 100644 --- a/pull_request_template.md +++ b/pull_request_template.md @@ -3,8 +3,8 @@ Put description here... # Code-Reviewer Section The general guidlines can be found [here](https://github.com/apple/foundationdb/wiki/FoundationDB-Commit-Process). -Please check each of the following things and check *all* boxed before accepting a PR. +Please check each of the following things and check *all* boxes before accepting a PR. -[ ] The PR has a description. +[ ] The PR has a description, explaining both the problem and the solution. [ ] The description mentions which forms of testing were done and the testing seems reasonable. [ ] Every function/class/actor that was touched is reasonably well documented. From 011648387f1a7f446f7f5cc50a3ada2479173b67 Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Fri, 12 Mar 2021 15:49:50 -0700 Subject: [PATCH 4/5] Fix typos Co-authored-by: A.J. Beamon --- pull_request_template.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pull_request_template.md b/pull_request_template.md index 0f5971ffbc..94be9ef790 100644 --- a/pull_request_template.md +++ b/pull_request_template.md @@ -1,9 +1,9 @@ Put description here... # Code-Reviewer Section -The general guidlines can be found [here](https://github.com/apple/foundationdb/wiki/FoundationDB-Commit-Process). +The general guidelines can be found [here](https://github.com/apple/foundationdb/wiki/FoundationDB-Commit-Process). -Please check each of the following things and check *all* boxed before accepting a PR. +Please check each of the following things and check *all* boxes before accepting a PR. [ ] The PR has a description. [ ] The description mentions which forms of testing were done and the testing seems reasonable. From 02248a8f9dd277cd4b63ece4054ba1b217e6c20c Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Mon, 15 Mar 2021 15:41:16 -0600 Subject: [PATCH 5/5] add release branch instructions --- pull_request_template.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pull_request_template.md b/pull_request_template.md index d409324fab..ddfe0499a6 100644 --- a/pull_request_template.md +++ b/pull_request_template.md @@ -1,6 +1,7 @@ Put description here... # Code-Reviewer Section + The general guidelines can be found [here](https://github.com/apple/foundationdb/wiki/FoundationDB-Commit-Process). Please check each of the following things and check *all* boxes before accepting a PR. @@ -8,3 +9,10 @@ Please check each of the following things and check *all* boxes before accepting [ ] The PR has a description, explaining both the problem and the solution. [ ] The description mentions which forms of testing were done and the testing seems reasonable. [ ] Every function/class/actor that was touched is reasonably well documented. + +## For Release-Branches + +If this PR is made against a release-branch, please also check the following: + +[ ] This change/bugfix is a cherry-pick from the next younger branch (younger `release-branch` or `master` if this is the youngest branch) +[ ] There is a good reason why this PR needs to go into a release branch and this reason is documented (either in the description above or in a linked GitHub issue)