Add a PR template #1007

julien-deramond · 2021-12-29T13:41:46Z

This PR proposes to add a PR template to avoid:

having to copy/paste DoD for feature or DoD for release
forgetting to do so

Moreover it will help the contributors to provide the needed information.

You can test it by creating a new pull request in https:/julien-deramond/github-repository-test/pulls (use the main-jd-fake-branch).

After the merge

Remove DoD for feature
Remove DoD for release

louismaximepiton · 2021-12-29T14:00:29Z

.github/PULL_REQUEST_TEMPLATE.md

+
+<!-- Describe your changes in detail -->
+
+### Motivation & Context


I'd regroup this part with the Related Issues (something like Motivation, Context and Related Issues)

Or move Related Issues up here or just before this paragraph ?

louismaximepiton · 2021-12-29T14:01:21Z

.github/PULL_REQUEST_TEMPLATE.md

+
+<!-- Please link any related issues here. -->
+
+### [Live preview](https://deploy-preview-{your pr number}--boosted.netlify.app/)


I'd move this part to the top, maybe just after description

.github/PULL_REQUEST_TEMPLATE.md

isabellechanclou · 2022-01-03T14:16:03Z

.github/PULL_REQUEST_TEMPLATE.md

+ - 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


Maybe remind the commands to run by adding : (using npm run ..., npm run ..., etc.)

isabellechanclou · 2022-01-03T14:16:30Z

.github/PULL_REQUEST_TEMPLATE.md

+ - 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


Maybe remind the commands to run by adding : (using npm run ..., npm run ..., etc.)

isabellechanclou · 2022-01-03T14:28:23Z

.github/PULL_REQUEST_TEMPLATE.md

+- [ ] Run linters
+- [ ] Run compilers
+- [ ] Tests added for JS-side
+- [ ] Run tests


Maybe remind the commands to run by adding : (using npm run test)

isabellechanclou · 2022-01-03T14:32:20Z

.github/PULL_REQUEST_TEMPLATE.md

+
+<!-- /!\ Core Team only: Uncomment the following for a release DoD -->
+<!--
+- [ ] Run linters;


Maybe remind the commands to run by adding : (using npm run ..., npm run ..., etc.)

isabellechanclou · 2022-01-03T14:32:33Z

.github/PULL_REQUEST_TEMPLATE.md

+<!-- /!\ Core Team only: Uncomment the following for a release DoD -->
+<!--
+- [ ] Run linters;
+- [ ] Run compilers;


Maybe remind the commands to run by adding : (using npm run ..., npm run ..., etc.)

isabellechanclou · 2022-01-03T14:32:53Z

.github/PULL_REQUEST_TEMPLATE.md

+<!--
+- [ ] Run linters;
+- [ ] Run compilers;
+- [ ] Run tests;


Maybe remind the commands to run by adding : (using npm run test)

isabellechanclou · 2022-01-03T14:34:47Z

.github/PULL_REQUEST_TEMPLATE.md

+ - IE11 (v4 only)
+ - Latest Edge, Chrome, Firefox, Safari
+ - iOS Safari
+ - Chrome & Firefox on Android


Have here the same list as above (if made suggestions are accepted)

isabellechanclou · 2022-01-03T14:42:20Z

.github/PULL_REQUEST_TEMPLATE.md

+
+<!-- Describe your changes in detail -->
+
+### Motivation & Context


Or move Related Issues up here or just before this paragraph ?

netlify · 2022-09-05T11:56:49Z

✅ Deploy Preview for boosted ready!

Name	Link
🔨 Latest commit	`2f6cc13`
🔍 Latest deploy log	https://app.netlify.com/sites/boosted/deploys/63204252da64f700086d7b27
😎 Deploy Preview	https://deploy-preview-1007--boosted.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

julien-deramond · 2022-09-05T11:58:58Z

I modified a bit the file architecture to gather what's been discussed in Bootstrap and here (/cc @louismaximepiton @MewenLeHo @isabellechanclou ).
I haven't added the corresponding npm run commands within the template for now. It could be done in a second PR after having tried this first version.
What do you think all? Do we start with that?

louismaximepiton · 2022-09-05T15:39:10Z

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.
Otherwise the actual version should do the work.

julien-deramond · 2022-09-06T05:51:08Z

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. Otherwise the actual version should do the work.

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.

MewenLeHo · 2022-09-06T07:20:55Z

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. Otherwise the actual version should do the work.

Should we do it? Maybe.
Can we do it? I honestly don't know. Most PRs are really different and about various subject. Would it be possible to find a common set of points to check?

julien-deramond · 2022-09-06T07:26:58Z

Should we do it? Maybe. Can we do it? I honestly don't know. Most PRs are really different and about various subject. Would it be possible to find a common set of points to check?

Most of the checks are common sense:

The PR corresponds to what's in the issue (if issue)
The PR doesn't break anything
Check all the possible non-regressions (not only the use case mentioned in the description)
Think about the users, migrations etc.
Write tests

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.

louismaximepiton · 2022-09-06T16:20:33Z

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.

chore(github): add a PR template

baf65b1

julien-deramond added chore meta labels Dec 29, 2021

julien-deramond changed the title ~~chore(github): add a PR template~~ Add a PR template Dec 29, 2021

louismaximepiton reviewed Dec 29, 2021

View reviewed changes

isabellechanclou reviewed Jan 3, 2022

View reviewed changes

Take into account discussions

96821f8

Merge branch 'main' into main-jd-add-pr-template

2f6cc13

julien-deramond merged commit 11752c0 into main Sep 13, 2022

julien-deramond deleted the main-jd-add-pr-template branch September 13, 2022 08:53

julien-deramond mentioned this pull request Sep 13, 2022

v5.2.1 Ship List #1429

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a PR template #1007

Add a PR template #1007

julien-deramond commented Dec 29, 2021 •

edited

Loading

louismaximepiton Dec 29, 2021 •

edited

Loading

isabellechanclou Jan 3, 2022

louismaximepiton Dec 29, 2021 •

edited

Loading

isabellechanclou Jan 3, 2022

isabellechanclou Jan 3, 2022

isabellechanclou Jan 3, 2022

isabellechanclou Jan 3, 2022

isabellechanclou Jan 3, 2022

isabellechanclou Jan 3, 2022

isabellechanclou Jan 3, 2022

isabellechanclou Jan 3, 2022

netlify bot commented Sep 5, 2022 •

edited

Loading

julien-deramond commented Sep 5, 2022

louismaximepiton commented Sep 5, 2022 •

edited

Loading

julien-deramond commented Sep 6, 2022

MewenLeHo commented Sep 6, 2022

julien-deramond commented Sep 6, 2022

louismaximepiton commented Sep 6, 2022


		<!-- Describe your changes in detail -->

		### Motivation & Context


		<!-- Please link any related issues here. -->

		### [Live preview](https://deploy-preview-{your pr number}--boosted.netlify.app/)

Add a PR template #1007

Add a PR template #1007

Conversation

julien-deramond commented Dec 29, 2021 • edited Loading

After the merge

louismaximepiton Dec 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

louismaximepiton Dec 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netlify bot commented Sep 5, 2022 • edited Loading

✅ Deploy Preview for boosted ready!

julien-deramond commented Sep 5, 2022

louismaximepiton commented Sep 5, 2022 • edited Loading

julien-deramond commented Sep 6, 2022

MewenLeHo commented Sep 6, 2022

julien-deramond commented Sep 6, 2022

louismaximepiton commented Sep 6, 2022

julien-deramond commented Dec 29, 2021 •

edited

Loading

louismaximepiton Dec 29, 2021 •

edited

Loading

louismaximepiton Dec 29, 2021 •

edited

Loading

netlify bot commented Sep 5, 2022 •

edited

Loading

louismaximepiton commented Sep 5, 2022 •

edited

Loading