chore(template): Added PR template #2993
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,43 @@ | ||||||
| <!-- Remove the sections that are not applicable --> | ||||||
|
|
||||||
| ## Describe your changes | ||||||
|
|
||||||
| <!-- Descriptions can consist of describing the change and/or images of the change --> | ||||||
|
|
||||||
| ## Checklist before requesting a review | ||||||
|
|
||||||
| - [ ] Is the PR title and first commit semantic? | ||||||
| - [ ] Have you checked if the test pass? | ||||||
|
Contributor
There was a problem hiding this comment.
Suggested change
|
||||||
| - [ ] Do the builds on github pass? | ||||||
|
Contributor
There was a problem hiding this comment.
Suggested change
Contributor
There was a problem hiding this comment. although should this be singular? |
||||||
|
|
||||||
| ## Checklist if you are making UI changes | ||||||
|
|
||||||
| - [ ] Did you verify the changes against the mocks? | ||||||
| - [ ] Are your changes a11y compliant? | ||||||
| - [ ] Are your changes responsive? (if applicable) | ||||||
|
|
||||||
|
Contributor
There was a problem hiding this comment. maybe we should ask them to test the changes in both storybook and at least one parent app
Contributor
There was a problem hiding this comment. actually maybe we should ask them to do that for ANY change, not just UI ones |
||||||
| ## Checklist if you are adding new stories | ||||||
|
|
||||||
| - [ ] Have you ran `yarn start:storybook` to manually test your story? | ||||||
| - [ ] Have you added a test for the stories that you added? | ||||||
| - [ ] Have you ran `yarn test:visuals` | ||||||
| - [ ] Did you ensure you only added necessary changes for adding a story? | ||||||
|
Contributor
There was a problem hiding this comment. I'm confused about what this one is asking |
||||||
|
|
||||||
| ## Checklist if you are converting JS files to TS | ||||||
|
Contributor
Author
There was a problem hiding this comment. Write new storybook/TS readmes ?
Contributor
There was a problem hiding this comment. potentially link them instead of including them here directly |
||||||
|
|
||||||
| <!-- Please follow the internal guide for conversion steps --> | ||||||
|
|
||||||
| - [ ] Did you delete the old snapshots if there were any? | ||||||
| - [ ] Did you convert the props to an interface? | ||||||
| - [ ] Did you fix the existing tests? | ||||||
| - [ ] Did you fill in the missing types? | ||||||
| - [ ] If the files you've migrated in `/components`, did you update `/components/index.ts` with exports and new components | ||||||
|
Contributor
There was a problem hiding this comment. I think there's a grammar error here, I think you might have meant to say "If the files you've migrated are in |
||||||
|
|
||||||
| ## Checklist JS -> TS before requesting a review | ||||||
|
Contributor
There was a problem hiding this comment. maybe we could merge this with the previous section: |
||||||
|
|
||||||
| - [ ] Are the code blocks in the components `tsx` spaced correctly? | ||||||
| - [ ] Did you run all the tests? | ||||||
| - [ ] Did you run `flow check`? | ||||||
|
greg-in-a-box marked this conversation as resolved.
|
||||||
| - [ ] Did you run `yarn lint`? | ||||||
| - [ ] If a story was added in conjunction with the conversion, did you run `yarn start:storybook`? | ||||||
| - [ ] Did you update `styleguide.config.js`? | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at minimum, the title needs to be semantic. I'm not 100% sure about the commit though.
this might not fit the format but should we link the commit guidelines? unnecessary if we're already doing that somewhere else. https://github.com/angular/angular.js/blob/master/DEVELOPERS.md#commits