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 #821

Merged
merged 5 commits into from
Dec 12, 2023
Merged

Create pull request template #821

merged 5 commits into from
Dec 12, 2023

Conversation

xpaczka
Copy link
Contributor

@xpaczka xpaczka commented Dec 1, 2023

Pull request template

This PR creates a pull request template that will be used for all pull requests, to make them consistent and always provide enough information about what was changes / what was added

The PR template looks like this:

Issues connected

Resolves

What has been done

  • Change 1

Testing

  • Test 1

Screenshots / images / videos

Please provide assets necessary for the PR (remove if not applicable)

Open to suggestions, what should be changed / what should be added

Copy link

netlify bot commented Dec 1, 2023

Deploy Preview for taho-development ready!

Name Link
🔨 Latest commit 9fb43b7
🔍 Latest deploy log https://app.netlify.com/sites/taho-development/deploys/657072166275360008d87670
😎 Deploy Preview https://deploy-preview-821--taho-development.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/workflows/create-pr-template.yml Outdated Show resolved Hide resolved
@xpaczka xpaczka marked this pull request as ready for review December 4, 2023 12:30
@xpaczka
Copy link
Contributor Author

xpaczka commented Dec 4, 2023

@ioay @jagodarybacka @Shadowfiend I'd appreciate your comment here, some improvement are also really welcomed :)

Copy link
Contributor

@michalinacienciala michalinacienciala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok to me now. Leaving for other folks to chime in.

Copy link
Contributor

@jagodarybacka jagodarybacka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what I would like to achieve with a PR template is both:

  • having unified structure of PR descriptions
  • improving or keeping the same the effort of creating the PR description while providing the value to PR reviewers

I would say that to achieve the points above we should aim to have to type as little text as possible while including as much context as possible.

I feel like this template contains too much text and includes details that are obfuscating the message of the PR.

My suggestions:

  • type the word "Resolves " instead of describing what this section is about - will save us time removing that text. In general I think we don't need to have the descriptions of these sections - either we will be removing them (additional time to remove that text) or we will leave them in the cretaed PR (additional effort for the reader to ignore that text while reading)
  • we just need one bullet point or checklist item prepared in the template to be able to add next by clicking "enter" while typing
  • "description" and "what has been done" are basically the same thing most of the time
  • why do we need to check the type of the PR? What kind of value is it providing? I believe it should be obvious looking at the title or description. I would find it valuable if we could filter by the type (so it would be a label for example) but idk if this is necessary
  • screenshots are optional and IMO they should be below the testing checklist (but its just my preference to keep the text content together and see the testing progress first)

In general I like the template idea but we should be careful while designing that thing to avoid adding more work than we had before 😄

@xpaczka xpaczka marked this pull request as draft December 6, 2023 10:58
Copy link
Contributor

@jagodarybacka jagodarybacka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it now, do we want to undraft?

@xpaczka xpaczka marked this pull request as ready for review December 12, 2023 08:54
@jagodarybacka jagodarybacka merged commit 1d63448 into main Dec 12, 2023
4 checks passed
@jagodarybacka jagodarybacka deleted the pr-template branch December 12, 2023 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants