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

Update PULL_REQUEST_TEMPLATE.md #574

Merged
merged 9 commits into from
Dec 13, 2022
Merged

Update PULL_REQUEST_TEMPLATE.md #574

merged 9 commits into from
Dec 13, 2022

Conversation

shaspitz
Copy link
Contributor

@shaspitz shaspitz commented Dec 8, 2022

Inspired by a talk @MSalopek and I just had. Only include things that're neccessary, merge agreements etc. should be defined elsewhere or enforced by reviewers and GH rules.

The Type of Change section will be important in reducing the effort it takes to review PRs. If a PR is a Feature and Refactor for example, this template will explicitly call out that the changes should likely be split up.

@shaspitz shaspitz requested a review from MSalopek December 8, 2022 19:52

# How was the feature tested?

- [ ] Unit tests
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe leave this in? Not sure if people are using it. If not, just delete :D

Copy link
Contributor Author

@shaspitz shaspitz Dec 8, 2022

Choose a reason for hiding this comment

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

Imo "how was this feature tested" is too broad. GH already shows you below which types of tests are passing or failing, and there's info in the main README about what each type of test is useful for.

The most important PR considerations in my mind are:

  1. what new or existing tests are acting as regression tests
  2. what new or existing tests are validating new behavior.

Just my two cents :), not trying to take things away that people like

@shaspitz shaspitz marked this pull request as ready for review December 13, 2022 18:15
@jtremback jtremback merged commit 4e60bbf into main Dec 13, 2022
@jtremback jtremback deleted the smaller-pr-template branch December 13, 2022 21:25
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