Skip to content

docs: update pull request template for improved clarity and formatting#200

Open
SalemBajjali wants to merge 3 commits intoga4gh:v1from
SalemBajjali:pr-template-update
Open

docs: update pull request template for improved clarity and formatting#200
SalemBajjali wants to merge 3 commits intoga4gh:v1from
SalemBajjali:pr-template-update

Conversation

@SalemBajjali
Copy link
Contributor

Resolves: #199

Updates the PR template to better reflect the current development workflow and generalize it. This approach was discussed with @brendanreardon during GKS-ThinkTank.

Before modifying the template, I reviewed the existing development workflow and organized the checklist to reflect the following steps:

  • Compile schema
  • Write/update tests
  • Document schema changes
  • Register new .rst files (if created)
  • Build docs locally

@SalemBajjali SalemBajjali requested a review from a team as a code owner February 11, 2026 21:11
@brendanreardon
Copy link
Collaborator

I LOVE this! Thank you so much, @SalemBajjali!

Do you think that it makes sense to wait for all products to approve the PR before simultaneously merging them? I can imagine benefits both ways. It could take a while to get each product to approve the PR, and not waiting could result in subsequent required PRs to make sure that all of the products are sync'd. To that end, I'm going to assign myself and @korikuzma as reviewers. If Kori is happy with the template then maybe she can make the executive decision to add it as is to all of the other repos.

Co-authored-by: Brendan Reardon <brreardon@gmail.com>
Copy link
Contributor

@korikuzma korikuzma left a comment

Choose a reason for hiding this comment

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

I think this is a good start! I have some minor suggestions

## Link to the corresponding Issue

Summary of the Pull Request.
## Summary of the Pull Request.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## Summary of the Pull Request.
## Summary of the Pull Request

## Summary of the Pull Request.

Pull Request checklist:
## Pull Request checklist:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## Pull Request checklist:
## Pull Request checklist

- [ ] Were the schema def/ and json/ files recompiled and committed? Run `cd schema; make all` from the root directory.
- [ ] If constraints or recipes were added, have they been added to the readthedocs? To do so, you can revise the appropriate file within `docs/source/concepts/`.
- [ ] Has documentation been regenerated and committed? Run `cd docs; make clean watch &` from the root directory to compile documentation.
### If the schema or examples were contributed to:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
### If the schema or examples were contributed to:
### Required if the schema or examples were contributed to

Comment on lines 8 to 11
- [ ] Does the title of this Pull Request reference the corresponding Issue?
- [ ] Is the branch validating against pre-commit hooks? Run `pre-commit run --all-files` from the root directory.
- [ ] Is the branch passing tests? Run `pytest tests/` from the root directory.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should phrase as a confirmation rather than question?

Suggested change
- [ ] Does the title of this Pull Request reference the corresponding Issue?
- [ ] Is the branch validating against pre-commit hooks? Run `pre-commit run --all-files` from the root directory.
- [ ] Is the branch passing tests? Run `pytest tests/` from the root directory.
- [ ] The title of this Pull Request accurately reflects the scope and content of the linked Issue.
- [ ] The branch passes all pre-commit hooks (Run `pre-commit run --all-files` from the root directory).
- [ ] The branch passes all tests (Run `pytest tests` from the root directory).

Comment on lines +13 to +17
- [ ] Were the schema `def/` and `json/` files recompiled and committed? Run `cd schema; make all` from the root directory.
- [ ] Have tests been created or updated?
- [ ] Have schema changes been documented (update existing files or create new ones in `docs/source/`)?
- [ ] If new `.rst` files were created, have they been registered in the documentation structure?
- [ ] Has documentation been regenerated and committed? Run `cd docs; make clean watch &` from the root directory to compile documentation.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above: I wonder if we should phrase as a confirmation rather than question?

Suggested change
- [ ] Were the schema `def/` and `json/` files recompiled and committed? Run `cd schema; make all` from the root directory.
- [ ] Have tests been created or updated?
- [ ] Have schema changes been documented (update existing files or create new ones in `docs/source/`)?
- [ ] If new `.rst` files were created, have they been registered in the documentation structure?
- [ ] Has documentation been regenerated and committed? Run `cd docs; make clean watch &` from the root directory to compile documentation.
- [ ] The schema `def/` and `json/` files have been recompiled and committed (Run `cd schema; make all` from the root directory).
- [ ] Tests have been created or updated.
- [ ] Schema changes have been documented (existing files updated or new files created in `docs/source/`).
- [ ] Any new `.rst` files have been registered in the documentation structure.
- [ ] Documentation has been regenerated and committed (Run `cd docs; make clean watch &` from the root directory).

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.

Improve pull request template

3 participants