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

Iterate on new Pull Request template to make PRs even easier to review (through additional clarity, context, and purpose) #40470

Open
adamziel opened this issue Apr 20, 2022 · 2 comments
Labels
[Type] Project Management Meta-issues related to project management of Gutenberg

Comments

@adamziel
Copy link
Contributor

adamziel commented Apr 20, 2022

What problem is this issue solving?

The following feedback to the new What? Why? How? PR template (introduced in #39229) was recently discussed in the #core-editor slack channel:

  • What? Why? How? may read abrupt
  • Achieving additional clarity seems easy
  • Simplify Pull Request template #39229 took 4 days to merge and there was no related core-editor discussion. A template so widely used would benefit from more inputs

As @youknowriad mentioned:

I think it's very hard to come up with the best PR template that doesn't discourage contributors or just make them ignore it. (As you can see in most of Gutenberg PRs), so I think we should iterate and see what works best, we know for certain that the current one doesn't work entirely.

Personally I see the new template as a big step in the right direction.

Now, the first feedback is in so let's discuss the next step.

What is the context?

The Pull Request template was recently shortened because the previous one:

  • Was quite verbose and not always followed
  • Did not clearly explain what the context and the purpose was

The new template is:

<!-- Thanks for contributing to Gutenberg! Please follow the Gutenberg Contributing Guidelines:
https://github.com/WordPress/gutenberg/blob/trunk/CONTRIBUTING.md -->

## What?
<!-- In a few words, what is the PR actually doing? -->

## Why?
<!-- Why is this PR necessary? What problem is it solving? Reference any existing previous issue(s) or PR(s), but please add a short summary here, too -->

## How?
<!-- How is your PR addressing the issue at hand? What are the implementation details? -->

## Testing Instructions
<!-- Please include step by step instructions on how to test this PR. -->
<!-- 1. Open a Post or Page. -->
<!-- 2. Insert a Heading Block. -->
<!-- 3. etc. -->

## Screenshots or screencast

Here's a few relevant comments from #39229:

@youknowriad said:

Answering the "Why", "How" and "What" can be a potential solution way to encourage contributors to add more context about their PRs, avoid the hidden assumptions and help them get more reviewers and feedback because it removes a burden from the reviewer to try to understand the reasoning and purpose of the PR. The reviewer doesn't need anymore to jump from linked issue to another to be fully operational on the PR.

@gziolo said:

I love the direction. In practice, as of today, the "Description" is treated like "What" from the new proposal. The new template proposed should help to gather more details. We too often miss the "Why" part which is crucial for understanding the reason was opened in the first place. The "How" section is nice to have as it can be digested from the code, but it is a great addition as it acts as a condensed summary of all code changes included. Maybe we should also ask there what alternatives were considered?

What solution does this issue propose?

Let's discuss:

  1. What are the goals of the PR template?
  2. What structure and words would best support these goals?

Rolling summary of the discussion

The high-level goals identified so far (April 20th):

  • Attract reviewers by making their job easy.
  • Support debugging by providing future readers with all the information they may need.

The tactical ways to achieve these goals:

  • Explain what problem is this PR solving to get the reviewer on board with the purpose.
  • Explain why the problem matters so the reviewers can a) prioritize b) get excited about getting it solved.
    • Summarize the relevant context so save the reviewers jumping between linked PRs and issues. Include any relevant visuals.
  • Summarize how does the PR solve the problem. Summarize the code changes so the reviewers don't have to spend 30 minutes figuring it out from scratch.
    • Say what's unintuitive to save time on the inevitable back-and-forth questions such as wait, what is this line doing?
    • Surface any hidden assumptions so reviewers can validate them in a discussion.
  • Say what feedback is needed so the reviewers can give you what you need.
    • Share what's out of scope to reduce drive-by discussions about extending the scope.
  • Explain how to verify that the PR solves the problem. Usually that's a test plan, but perhaps not always?
  • Attach visuals to support the points above, explain complex ideas with ease, and make the PR fun and easy to work with.

What’s unintuitive?

A template too short doesn't draw out enough information. A template too long feels overwhelming and gets ignored. A goldilocks template draws out just enough information to help both the writer and the reader. Let's aim for that.

What feedback is needed?

First, let's identify and prioritize the goals. Only then let's look for the right words to achieve them.

Also, please ping and loop in more folks.

cc @bph @Mamaduka @carolinan @mcsf @priethor @annezazu @getdave @gwwar @hypest @draganescu @scruffian @ndiego @noisysocks @michalczaplinski @hellofromtonya @mtias @talldan @paaljoachim @dmsnell

Please absolutely feel free to loop in more folks and let's surface this on the core-editor chat

@adamziel adamziel added the [Type] Project Management Meta-issues related to project management of Gutenberg label Apr 20, 2022
@adamziel adamziel changed the title Iterate on the PR template for even more clarity, context, purpose Iterate on the PR template to provide even more clarity, context, purpose in new Pull Requests (so they're easier to review) Apr 20, 2022
@adamziel adamziel changed the title Iterate on the PR template to provide even more clarity, context, purpose in new Pull Requests (so they're easier to review) Iterate on the PR template to make new Pull Requests even easier to review (with additional clarity, context, and purpose) Apr 20, 2022
@adamziel adamziel changed the title Iterate on the PR template to make new Pull Requests even easier to review (with additional clarity, context, and purpose) Iterate on the PR template to make new Pull Requests even easier to review (through additional clarity, context, and purpose) Apr 20, 2022
@adamziel adamziel changed the title Iterate on the PR template to make new Pull Requests even easier to review (through additional clarity, context, and purpose) Iterate on new Pull Request template to make PRs even easier to review (through additional clarity, context, and purpose) Apr 20, 2022
@mtias
Copy link
Member

mtias commented Apr 21, 2022

Share all the context

I don't think this should be a goal as it won't be reasonable to achieve. I think PRs should be more about proposing and articulating an implementation — the context of which should be captured and discussed in issues. For example, I'd expect PRs to include screenshots or videos of the implementation, not to repeat the mockups that should be present with more depth on an issue.

@adamziel
Copy link
Contributor Author

@mtias Good call! The word "all" may indeed suggest bringing in all the intermediate mockups and such that aren't even relevant for a particular PR. I replaced it with "Summarize the relevant context".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Project Management Meta-issues related to project management of Gutenberg
Projects
None yet
Development

No branches or pull requests

2 participants