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.

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

Create pull request template #43

Merged
merged 4 commits into from Jan 12, 2020
Merged

Conversation

@khanguslee
Copy link
Member

khanguslee commented Jan 9, 2020

A fairly simple PR template. Am I missing anything else? Once we finalise on a template, I will create the same template for all repos!

Copy link

Blake-Haydon left a comment

Looks good, just want to check that its only 7 lines

Copy link
Member

hallgchris left a comment

For inspiration, here's Rome2rio's PR template

[removed 17/01/2020]

Quite extra, and no one uses the checkboxes anyway, but I feel like we could take a bit from it. I find that although it's nice to think that we'd remember to do things like include screenshots or whatever, it's easy to forget and I do find it useful to have right in front of me.

Perhaps in the Description section we could include a reminder that there should be screenshots if relevant, and state that there should be one conceptual change (if that can be tied into the wording in some non-awkward way).

In the testing section, a small reminder to write automated tests, if relevant, could also be good.

Finally, what do you think about a pre-merge checklist? Could be all on one line, specifying that all feedback should be addressed, will squash and merge, whatever.

Idk, maybe this is all too extra 😅 thoughts people?

Also if I remember, I'll remove the excerpt from Rome2rio's repo in a few days. Probably not the best idea go around posting that

@khanguslee

This comment has been minimized.

Copy link
Member Author

khanguslee commented Jan 10, 2020

Definitely agree with the checkboxes as we will just click it without reading it.

Perhaps in the Description section we could include a reminder that there should be screenshots if relevant, and state that there should be one conceptual change (if that can be tied into the wording in some non-awkward way).

My workplace does that atm as well, so I'll add a screenshots section (if relevant)

In the testing section, a small reminder to write automated tests, if relevant, could also be good.

Unfortunately we aren't at the stage where we have automated testing. With Github Actions/Travis this will be done automatically anyway.

Finally, what do you think about a pre-merge checklist? Could be all on one line, specifying that all feedback should be addressed, will squash and merge, whatever.

I think this should be known already by submitting a PR.

Good suggestions though!

Copy link
Member

hallgchris left a comment

Add the screenshots section and LGTM

khanguslee added 2 commits Jan 11, 2020
…human-power/dashboard into khanguslee/pull-request-template
@khanguslee khanguslee merged commit 19c8215 into master Jan 12, 2020
1 check failed
1 check failed
Codacy/PR Quality Review Not up to standards. This pull request quality could be better.
Details
@khanguslee khanguslee deleted the khanguslee/pull-request-template branch Jan 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.