Add a PR template #1007
Add a PR template #1007
Conversation
|
|
||
| <!-- Describe your changes in detail --> | ||
|
|
||
| ### Motivation & Context |
There was a problem hiding this comment.
I'd regroup this part with the Related Issues (something like Motivation, Context and Related Issues)
There was a problem hiding this comment.
Or move Related Issues up here or just before this paragraph ?
.github/PULL_REQUEST_TEMPLATE.md
Outdated
|
|
||
| <!-- Please link any related issues here. --> | ||
|
|
||
| ### [Live preview](https://deploy-preview-{your pr number}--boosted.netlify.app/) |
There was a problem hiding this comment.
I'd move this part to the top, maybe just after description
| - in /forms/validation/?#supported-elements if it is a new Orange custom component that is a form control | ||
| - in /forms/overview/ if it is a new Orange custom component that is a form control | ||
| - [ ] Check (and fix) RTL version | ||
| - [ ] Run linters |
There was a problem hiding this comment.
Maybe remind the commands to run by adding : (using npm run ..., npm run ..., etc.)
| - in /forms/overview/ if it is a new Orange custom component that is a form control | ||
| - [ ] Check (and fix) RTL version | ||
| - [ ] Run linters | ||
| - [ ] Run compilers |
There was a problem hiding this comment.
Maybe remind the commands to run by adding : (using npm run ..., npm run ..., etc.)
| - [ ] Run linters | ||
| - [ ] Run compilers | ||
| - [ ] Tests added for JS-side | ||
| - [ ] Run tests |
There was a problem hiding this comment.
Maybe remind the commands to run by adding : (using npm run test)
|
|
||
| <!-- /!\ Core Team only: Uncomment the following for a release DoD --> | ||
| <!-- | ||
| - [ ] Run linters; |
There was a problem hiding this comment.
Maybe remind the commands to run by adding : (using npm run ..., npm run ..., etc.)
| <!-- /!\ Core Team only: Uncomment the following for a release DoD --> | ||
| <!-- | ||
| - [ ] Run linters; | ||
| - [ ] Run compilers; |
There was a problem hiding this comment.
Maybe remind the commands to run by adding : (using npm run ..., npm run ..., etc.)
| <!-- | ||
| - [ ] Run linters; | ||
| - [ ] Run compilers; | ||
| - [ ] Run tests; |
There was a problem hiding this comment.
Maybe remind the commands to run by adding : (using npm run test)
| - IE11 (v4 only) | ||
| - Latest Edge, Chrome, Firefox, Safari | ||
| - iOS Safari | ||
| - Chrome & Firefox on Android |
There was a problem hiding this comment.
Have here the same list as above (if made suggestions are accepted)
|
|
||
| <!-- Describe your changes in detail --> | ||
|
|
||
| ### Motivation & Context |
There was a problem hiding this comment.
Or move Related Issues up here or just before this paragraph ?
✅ Deploy Preview for boosted ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
I modified a bit the file architecture to gather what's been discussed in Bootstrap and here (/cc @louismaximepiton @MewenLeHo @isabellechanclou ). |
|
An idea just popped up to my mind, should we add a checklist for reviewers? I mean something to be sure that we aren't missing something obvious as reviewer? It doesn't prevent to check some other things that were not noticed, and could be filled up by other reviewers. |
IMO the checklist for the reviewer is the same as the one for the person proposing the PR; you got to be your own reviewer when you propose your PR. |
Should we do it? Maybe. |
Most of the checks are common sense:
IMO the common part is rather obvious and the rest is, as @MewenLeHo said, "really different and about various subject" and depending on the context. |
|
Let's merge it this way! As discussed with @julien-deramond, could come later but could be useful for external contributors or newcomers to have at least a base for their own review. |
This PR proposes to add a PR template to avoid:
Moreover it will help the contributors to provide the needed information.
This is based on twbs/bootstrap#35079.
You can test it by creating a new pull request in https://github.com/julien-deramond/github-repository-test/pulls (use the main-jd-fake-branch).
After the merge