[github]: add pull request template

[saehejkang]

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update

Motivation and Context

• Adds a pull request template

Testing

• Tested locally
• Added/updated tests
• Added/updated docs

[jglogan]

@saehejkang lol I kind of want to get rid of the one over in container 😄

Or refine what's there. Let's keep this open for now and I'll take it up with the other maintainers today.

[katiewasnothere]

I agree with @jglogan :/

I feel like the only reason I'm okay with the template in container is that that's where we get most of our contributions and it's nice to remind people to write good PR descriptions and test things. I really wish GitHub had PR templates that were similar to issue templates so we could have more complex fields.

My main issues with the PR templates as they are today are:

1. We use squashed commits which then uses the PR description as the final commit message once the commits are squashed. The template makes those commits probably less clear than if we just kept the commit messages as they were from the PR. But the issue with that is that not everyone knows how to write good commit messages and we don't want to be staling PRs on the author needing to just change the commit messages.
2. We can't add conditional fields. It'd be nice if we could have something like if someone checks that they added a new feature that they're REQUIRED to also check that they have added tests.
3. We can't add mandatory fields. I like that in the issue templates we can require that people agree to the code of conduct for the project.
4. The GitHub UI treats the checkboxes in a PR description as like "todo" task items, so when you're looking at the list of PRs (https://github.com/apple/container/pulls), they'll say something like "1 of 7 tasks", which is unclear and annoying.

[jglogan]

@katiewasnothere Re #1, I wind up editing all commit messages that I merge for conciseness (except where I forget to).

My opinion is that it's okay for maintainers to have a bit of editorial control on the commit messages. We're not mucking with the content of the PRs and it feels nitpicky and inefficient for us to go back and forth with contributors over the messages.

The task lists convey very little info to me. Would it work to put our "PR submission reminders" (write tests! write documentation!) <!-- in comments? --> I think they may appear when opening the PR but will get filtered for display.

[katiewasnothere]

@jglogan Yeah I definitely think maintainers can have some editorial control on the final commit message. It's more just annoying that we need to edit the final commit message and we may forget.

I would prefer we not put the reminders in a comment in the template. I like that the checkbox guides people to explicitly confirm they've done something like add tests, etc.

[saehejkang]

I have read all the comments and may have an idea on a solution that solves all these issues 👀 . For a first pass, I will only focus on two types of PRs: a bug fix or new feature. In a way, this solution could also provide a new way to determine when a PR is FULLY COMPLETE before review, since we want the checkbox guides to be completed. Let me push something up here in the next couple of days and we can continue the discussion then.

[saehejkang]

@jglogan @katiewasnothere I pushed changes for a bug fix template. The other templates will be very similar, so it warrants a review before I continue. I hope this is sort of in the right direction, as it allows template tailoring for different types of PRs (bugs, features, refactoring, etc.)