From 2b4888e0fd76ec1cd5c820e31f9322bf0fb00176 Mon Sep 17 00:00:00 2001 From: gerbus Date: Tue, 6 Oct 2020 08:02:15 -0700 Subject: [PATCH 1/2] Copy relevant updates from template in janeapp/jane --- PULL_REQUEST_TEMPLATE.md | 41 ++++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/PULL_REQUEST_TEMPLATE.md b/PULL_REQUEST_TEMPLATE.md index bfc1358e3b7e..d17dd79bd004 100644 --- a/PULL_REQUEST_TEMPLATE.md +++ b/PULL_REQUEST_TEMPLATE.md @@ -2,27 +2,24 @@ > Please make your PR readme as readable as possible. Only keep parts of this template that are filled out and meaningful to the reviewer. > > - Remove any quoted parts of the template below `>`. (These are just instructions for you to follow) -> - Remove any sections that are irrelevant. +> - Leave all headings, but clear content and mark as `N/A` if irrelevant. > - Most importantly: explain the "WHY". Do your best to give the reviewer some context about this change. ## Description -> Provide a summary of the problem being solved, the work done, and what was added/removed/changed - - [Jira Ticket](https://janeapp.atlassian.net/browse/...) -### Risk / General PR Class -- [ ] 🔴 = High Risk -- [ ] 🔶 = Medium Risk -- [ ] 💚 = Low Risk -- [ ] 💜 = Almost Zero Risk -- [ ] 0️⃣ = Zero Risk +> Provide a summary of the problem being solved, the work done, and what was added/removed/changed +### General PR Class 🐛 = Bug Fix (Fixes an Issue) 🌟 = New Feature (Adds Functionality) 🏇 = Performance improvement 👁 = UX / UI improvement 🏗 = Refactor +💾 = Database Migrations +🔎 = SQL Views 🌦 = Env Changes +💎 = Ruby Gem Changes ☕️ = JS Dependency Changes ⚛️ = Jane Desktop Changes 💻 = Browser Refresh Required (Unobtrusive, Immediate, or Forced) @@ -33,11 +30,35 @@ ### Dependencies / ENV / Migrations / Client Reset > Describe any migrations, dependencies or ENV variables that are required for this change. Add notes regarding the release such as rake tasks, libraries, or the need for a client reset. Notify the team, if they have to update their environment. +### Risk Scorecard +> 1. As the author you should check the boxes that correspond with your PR and then use the following guide to set your risk label: +> * 0 checkboxes => low risk +> * 1-3 checkboxes => medium risk +> * 4+ checkboxes => high risk +> 2. Unless exempt, checked risk factors should be explained comprehensively in the Release Risk Assessment section below +> 3. Medium or higher risk PRs should get more than one code-review approval +> +> NOTE: if you aren't changing any production files, please use the zero risk label + +- [ ] migrations or view changes1 +- [ ] browser refresh required +- [ ] cannot be rolled back (tally tokens, view or migration changes, API changes) +- [ ] could create bad data +- [ ] requires env configuration to be added in production +- [ ] gemfile or js package changes1 +- [ ] more than 200 LOC changed in production files1 +- [ ] includes a user-facing workflow change to an existing production feature (user muscle memory or pattern recognition will be affected) +- [ ] could prevent access to Jane (eg. cors, middleware, changes to auth system) +- [ ] affects a widely used component or piece of code +- [ ] I have a doubt - I want the RMT to review this. If possible, please elaborate your concerns in the risk assessment section. + +1 No need to explain these risk factors below + ### Release Risk Assessment > Describe what areas of Jane are touched by the change in this PR, and what it would look like if something were to go wrong, and how much damage could be done. Keep your neighborhood deployer in mind when filling in this section, it will help identify errant PRs more quickly during deploy. ### Demo Notes -> If you have instructions on how to demo, or a video/gif, add it here +> If you have instructions on how to demo or a video add it here ## Code Review Resource: [Dev Team Notion Page](https://www.notion.so/janeapp/Dev-Team-f06c6eb2ccca4066bc63fc1ac1bd2549) From 00d705fba917c8c421d67e7c2da80f373e6a7673 Mon Sep 17 00:00:00 2001 From: gerbus Date: Tue, 6 Oct 2020 08:10:29 -0700 Subject: [PATCH 2/2] Remove parts irrelevant to this repo --- PULL_REQUEST_TEMPLATE.md | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/PULL_REQUEST_TEMPLATE.md b/PULL_REQUEST_TEMPLATE.md index d17dd79bd004..733f05ea5ce8 100644 --- a/PULL_REQUEST_TEMPLATE.md +++ b/PULL_REQUEST_TEMPLATE.md @@ -16,19 +16,15 @@ 🏇 = Performance improvement 👁 = UX / UI improvement 🏗 = Refactor -💾 = Database Migrations -🔎 = SQL Views 🌦 = Env Changes -💎 = Ruby Gem Changes ☕️ = JS Dependency Changes ⚛️ = Jane Desktop Changes -💻 = Browser Refresh Required (Unobtrusive, Immediate, or Forced) ### Release Note > Describe here a CS friendly version of what this fixes/adds. If the change is behind a feature flag, please include that in your description. If this is a hidden change CS doesn't need to really know about then just say so. -### Dependencies / ENV / Migrations / Client Reset -> Describe any migrations, dependencies or ENV variables that are required for this change. Add notes regarding the release such as rake tasks, libraries, or the need for a client reset. Notify the team, if they have to update their environment. +### Dependencies / ENV +> Describe any dependencies or ENV variables that are required for this change. Notify the team, if they have to update their environment. ### Risk Scorecard > 1. As the author you should check the boxes that correspond with your PR and then use the following guide to set your risk label: @@ -40,15 +36,11 @@ > > NOTE: if you aren't changing any production files, please use the zero risk label -- [ ] migrations or view changes1 -- [ ] browser refresh required -- [ ] cannot be rolled back (tally tokens, view or migration changes, API changes) -- [ ] could create bad data - [ ] requires env configuration to be added in production -- [ ] gemfile or js package changes1 +- [ ] js package changes1 - [ ] more than 200 LOC changed in production files1 - [ ] includes a user-facing workflow change to an existing production feature (user muscle memory or pattern recognition will be affected) -- [ ] could prevent access to Jane (eg. cors, middleware, changes to auth system) +- [ ] could prevent access to Jane Video (eg. cors, middleware, changes to auth system) - [ ] affects a widely used component or piece of code - [ ] I have a doubt - I want the RMT to review this. If possible, please elaborate your concerns in the risk assessment section. @@ -68,7 +60,7 @@ Resource: [Code Review Checklist](https://www.notion.so/janeapp/Code-Review-chec #### Design - [ ] I added instructions for how to test, in the QA section below -- [ ] I added tests for changes, or determined that none were required +- [ ] I added specs for changes, or determined that none were required - [ ] I demoed this to the appropriate person - [ ] I considered both mobile & desktop views, or that wasn't relevant @@ -77,8 +69,6 @@ Resource: [Code Review Checklist](https://www.notion.so/janeapp/Code-Review-chec - [ ] I wrote readable code, or added comments if it was complex - [ ] I performed a self-review of my own code - [ ] I rebased my branch on the latest `master` -- [ ] I confirmed that all CircleCI tests are passing -- [ ] I didn't add new npm packages, or else I made damn sure the `yarn.lock` changes are safe for existing production packages ## QA and Smoke Testing ### Steps to Reproduce