Update pull request template to add integration tests with github/github checklist item #3844
Conversation
…hub checklist item
|
size-limit report 📦
|
broccolinisoup
requested review from
a team,
joshblack,
langermank,
pksjce and
siddharthkp
broccolinisoup
temporarily deployed
to
github-pages
GitHub Actions
Inactive
broccolinisoup
temporarily deployed
to
github-pages
GitHub Actions
Inactive
.github/pull_request_template.md
Outdated
| - [ ] Integration tests passed at github/github (Optional and only available for GitHub staff) | ||
|
|
||
| You can refer to the [testing primer react pull requests documentation](https://github.com/github/primer-engineering/blob/main/how-we-work/testing-primer-react-pr-at-dotcom.md) to learn how to run integrations test at github/github. |
There was a problem hiding this comment.
This might be more of a PR template question, but wanted to ask what would the difference be in adding this to Testing & Reviewing vs Merge checklist?
One way I've been using this section (Testing & Reviewing) is to try and give step-by-step instructions for how to test or review the PR for reviewers and the instructions tend to be fairly different across PRs. If others use it similarly, maybe this is more of a merge checklist item since it's going to be fairly common and repeated across PRs? Curious what you think
There was a problem hiding this comment.
Thanks for your comment Josh!
wanted to ask what would the difference be in adding this to Testing & Reviewing vs Merge checklist
I wasn't sure which bucket would be the best really and the way you utilize Testing & Reviewing section kind of clarifies the answer for me. I am happy to take it down to merge checklist!
.github/pull_request_template.md
Outdated
|
|
||
| <!-- Describe any specific details to help reviewers test or review this Pull Request --> | ||
|
|
||
| - [ ] Integration tests passed at github/github (Optional and only available for GitHub staff) |
There was a problem hiding this comment.
Just wanted to suggest an alternative to see how it landed, goal was to see how this could be a checklist item and if it could capture relevant info for people who hadn't seen it before:
| - [ ] Integration tests passed at github/github (Optional and only available for GitHub staff) | |
| - [ ] (Maintainers) Integration tests pass at github/github ([Learn more](https://github.com/github/primer-engineering/blob/main/how-we-work/testing-primer-react-pr-at-dotcom.md)) |
There was a problem hiding this comment.
This looks good - I think we would only need to update the "Learn more" link text to avoid generic link text (https://www.w3.org/TR/UNDERSTANDING-WCAG20/navigation-mechanisms-refs.html)
|
@joshblack pushed another commit to address your comment - let me know what you think! Feel free to suggest changes if there is anything missing. |
Closes #
Changelog
Based on the release improvements discussion, team agreed to add another checklist item for running (optional) integration tests per pr at github/github.
New
Added a checklist item under the "Testing & Reviewing" section of the pull request template for integration tests at github/github. Also linked its documentation that includes the motivation of running integration tests as well as step by step guide how to use the GH action to make sure the pr doesn't introduce any breaking changes to github/github
Changed
Removed
Rollout strategy
Skip changeset
Testing & Reviewing
Confirming that the new checklist appears in the pull request template after merge
Merge checklist
Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.