[Merged by Bors] - Add PR Template

[NathanSWard]

This is a first step at addressing #2256 via adding a pr template.

[NathanSWard]

It's a bummer that the PR templates don't support the yaml config at the top of them template so we can auto populate labels and what not....

[alice-i-cecile]

It's a bummer that the PR templates don't support the yaml config at the top of them template so we can auto populate labels and what not...

They do ;) Check https://github.com/Leafwing-Studios/template-repo

[NathanSWard]

They do ;) Check https://github.com/Leafwing-Studios/template-repo

Are you sure?
I was looking at this open issue and they said it doesn't support it (unless it changed and I'm not aware of it...)
isaacs/github#1252 (comment)

[alice-i-cecile]

Yep I've used it in my own projects and it sets the metadata correctly. The web form didn't support it though, so I wrote it by hand like https://github.com/Leafwing-Studios/template-repo/blob/main/.github/PULL_REQUEST_TEMPLATE/rfc.md

[mockersf]

I would prefer a much simpler, minimal template... I often find them painful

Either it's someone who is used to PRs/contributing and the template is useless as they already write good enough PRs, or it's someone that's more new and this adds friction

Sorry, forgot to mention that in #2256...

[NathanSWard]

Sorry, forgot to mention that in #2256...

No worries 🙂

Either it's someone who is used to PRs/contributing and the template is useless as they already write good enough PRs, or it's someone that's more new and this adds friction

This is fair, however I would say this is more about consistency and making sure there's a standard for PRs. There are a slew of people who make great PRs, however, occasionally a not a nice one can slip though.
Also there are people in the middle: those who are not first time contributors and who don't always have the most informative/relevant PR descriptions.

Also the template is super helpful when you're reviewing PRs, as you can easily see what the PR is accomplishing and it's organized which is a huge plus. Also it should hopefully reduce how often we need to remind contributors to add tests/docs/examples when necessary haha 😅

And hopefully this template isn't too painful 😣, is there a specific part that seems unnecessary?

[mockersf]

Also it should hopefully reduce how often we need to remind contributors to add tests/docs/examples when necessary haha

But for now at least, we don't want to enforce the need for tests/docs/examples. Yes it is better to always have those, but tests should only be mandatory when adding something tricky, and doc/examples when adding an interesting feature. Again, this is only for now and may change later. Just having "if applicable" doesn't transmit that in my opinion.

For the mid-term future, our issue is not having a lot of bad quality PRs, or not enough community members to review them I think.

consistency and making sure there's a standard for PRs

My momma always said, “Reviewing a PR was like a box of chocolates. You never know what you're gonna get.”

[alice-i-cecile]

For the mid-term future, our issue is not having a lot of bad quality PRs, or not enough community members to review them I think.

Agreed. I think the big issues we have are a) organizing and prioritizing work b) reviewing can be too intimidating

For b), I think this is caused by:

1. Missing docs / poor code quality in existing code (hi bevy_ecs, bevy_render and bevy_ui)
2. Perceived barriers to entry
3. Very terse PR descriptions or issues that don't describe the intent or strategy of the change well
4. Extremely complex new features that aren't explained adequately in the PR description, RFC or the code

[NathanSWard]

But for now at least, we don't want to enforce the need for tests/docs/examples

Granted not every PR will need any/all of them, however for those PRs where they do fit, we should require them.
Not adding tests when necessary makes it easy to break functionality on accident in the future, not having enough docs/examples just adds to our technical debt of not having enough doc/examples lol.

I'm still a fan of having it in the PR template, though if the majority (or @cart) disagrees, I'm ok with removing it as well.

For the mid-term future, our issue is not having a lot of bad quality PRs, or not enough community members to review them I think.

120% agree with there not enough reviewers. This is also why I'm especially thankful to you (@mockersf ) and @alice-i-cecile for being very consistent with the PR reviews ♥️ (and always responding when I request a review lol)

My momma always said, “Reviewing a PR was like a box of chocolates. You never know what you're gonna get.”

🌲🌲Gump

[cart]

I'm honestly not super convinced that we need a text template here. I agree that right now we don't want to require tests/examples/docs. I also haven't encountered a pr where the author didn't provide the context needed, and we can always ask for clarity. And imo reviewers should use context in the pr to determine if/when tests/examples/docs are needed. Not including a template is a small thing we can do to cut down on the stress and cognitive load new contributors experience when creating a pr.

Rust gets away without having a pr text template. I think we can too.

[NathanSWard]

I agree that right now we don't want to require tests/examples/docs.

Sure I'm fine removing the checklist section

Not including a template is a small thing we can do to cut down on the stress and cognitive load new contributors experience when creating a pr.

Umm I would actually argue that it would help new contributors.
If someone is creating a PR for this first tIme all they're given is a blank description box, this is actually more daunting (imo) since they don't have any idea what to write there. (This is similar to when you're writing a school essay and all you have is a blank page and this often makes you feel overwhelmed).

Whereas, if we have a simple starting template with 3 sections

1. Objective
2. Solution
3. Additional Information

And a few notes on what to put there it would more easily allow the contributor to organize their thoughts.

This is at least my take on PR templates.

[alice-i-cecile]

If someone is creating a PR for this first tIme all they're given is a blank description box, this is actually more daunting (imo) since they don't have any idea what to write there. (This is similar to when you're writing a school essay and all you have is a blank page and this often makes you feel overwhelmed).

That's definitely how my brain works ;) I'm sure we have a heterogenous population here though, just like we saw with RFCs.

[cart]

Alrighty as long as its simple I'm sold. Can we remove the "additional information" section? Imo people will already add whatever information they think is relevant. No need to force a particular structure on them.

[NathanSWard]

Can we remove the "additional information" section?

Sure!

[mockersf]

as it's now shorter, could you set it back as a markdown and add newlines after titles to pass validation? It should still fit in the new PR textbox

[NathanSWard]

as it's now shorter, could you set it back as a markdown and add newlines after titles to pass validation? It should still fit in the new PR textbox

done :)

[cart]

bors r+

[bors]

Pull request successfully merged into main.

Build succeeded:

• build-android
• build (nightly, ubuntu-latest)
• build (stable, macos-latest)
• build (stable, ubuntu-latest)
• build (stable, windows-latest)
• build-wasm (nightly, ubuntu-latest)
• build-wasm (stable, ubuntu-latest)
• check-doc
• check-markdown-links
• check-missing-examples-in-docs
• markdownlint
• run-examples