Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Pull Request Template #23

Merged
merged 2 commits into from
Oct 13, 2020
Merged

Update Pull Request Template #23

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2b4888e
Copy relevant updates from template in janeapp/jane
gerbus Oct 6, 2020
00d705f
Remove parts irrelevant to this repo
gerbus Oct 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 27 additions & 16 deletions PULL_REQUEST_TEMPLATE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,15 @@
> 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
Expand All @@ -25,19 +19,38 @@
🌦 = Env 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:
> * 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

- [ ] requires env configuration to be added in production
- [ ] js package changes<sup>1</sup>
- [ ] more than 200 LOC changed in production files<sup>1</sup>
- [ ] 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 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.

<sup>1</sup> 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)
Expand All @@ -47,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

Expand All @@ -56,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
Expand Down