diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index b4ccdd91554..8deb0fe4695 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,33 +1,36 @@ -## Description + - +## Context - +- Describe the objective or issue this PR addresses, as well as the impact of the change. +- Try to keep the description accessible to newcomers. +- If you're resolving a specific issue, add "Fixes #issue_number" or "Closes #issue_number". -### Linked issue +### Solution - +- Describe the approach taken to achieve the objective / resolve the issue. -Closes #{issue_number} +### Migration Guide (optional) - +- If this PR contains a breaking change relative to the `main` branch, provide an instruction on how affected parties might need to adapt to the change. -### Benefits +--- - +### Review notes (optional) + +- For complex PRs, try to provide some information on how to approach the review more effectively. +- For example, is there a natural order in which the affected files should be reviewed? ### Checklist -- [ ] I've read `CONTRIBUTING.md` -- [ ] I've used the standard signed-off commit format (or will squash just before merging) -- [ ] All applicable CI checks pass (or I promised to make them pass later) -- [ ] (optional) I've written unit tests for the code changes -- [ ] I replied to all comments after code review, marking all implemented changes with thumbs up +- [ ] I've read [`CONTRIBUTING.md`](../CONTRIBUTING.md). +- [ ] (optional) I've written unit tests for the code changes. +- [ ] All review comments have been resolved. - + + - Commit sign-off: https://www.secondstate.io/articles/dco + - Telegram: https://t.me/hyperledgeriroha + - Discord: https://discord.com/channels/905194001349627914/905205848547155968 +--> \ No newline at end of file diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7b146314194..a173b877fcb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -127,27 +127,33 @@ You, as part of the aforementioned community, should consider helping others too Please [fork](https://docs.github.com/en/get-started/quickstart/fork-a-repo) the [repository](https://github.com/hyperledger/iroha/tree/main) and [create a feature branch](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-and-deleting-branches-within-your-repository) for your contributions. When working with **PRs from forks**, check [this manual](https://help.github.com/articles/checking-out-pull-requests-locally). -Working on code contribution: +#### Working on code contribution: - Follow the [Rust Style Guide](#rust-style-guide) and the [Documentation Style Guide](#documentation-style-guide). - Ensure that the code you've written is covered by tests. If you fixed a bug, please turn the minimum working example that reproduces the bug into a test. -Committing your work: +#### Committing your work: - Follow the [Git Style Guide](#git-workflow). - Squash your commits [either before](https://www.git-tower.com/learn/git/faq/git-squash/) or [during the merge](https://rietta.com/blog/github-merge-types/). - If during the preparation of your pull request your branch got out of date, rebase it locally with `git pull --rebase upstream main`. Alternatively, you may use the drop-down menu for the `Update branch` button and choose the `Update with rebase` option. In the interest of making this process easier for everyone, try not to have more than a handful of commits for a pull request, and avoid re-using feature branches. -Creating a pull request: +#### Creating a pull request: - Use an appropriate pull request description by filling in the [description template](.github/PULL_REQUEST_TEMPLATE.md). Avoid deviating from this template if possible. - Add an appropriately formatted [pull request title](#pull-request-titles). - If you feel like your code isn't ready to merge, but you want the maintainers to look through it, create a draft pull request. -Merging your work: +#### Merging your work: - A pull request must pass all automated checks before being merged. At a minimum, the code must be formatted, passing all tests, as well as having no outstanding `clippy` lints. - A pull request cannot be merged without two approving reviews from the active maintainers. - Each pull request will automatically notify the code owners. An up to date list of current maintainers can be found in [MAINTAINERS.md](MAINTAINERS.md). +#### Review etiquette: +- Do not resolve a conversation on your own. Let the reviewer make a decision. +- Acknowledge review comments and engage with the reviewer (agree, disagree, clarify, explain, etc.). Do not ignore comments. +- For simple code change suggestions, if you apply them directly, you can resolve the conversation. +- Avoid overwriting your previous commits when pushing new changes. It obfuscates what changed since the last review and forces the reviewer to start from scratch. Commits are squashed before merging automatically. + ### Pull Request Titles We parse the titles of all the merged pull requests to generate changelogs. We also check that the title follows the convention via the *`check-PR-title`* check. @@ -191,7 +197,6 @@ Follow these commit guidelines: - Try to stick to one commit per meaningful change. - If you fixed several issues in one PR, give them separate commits. - As mentioned previously, changes to the `schema` and the API should be done in appropriate commits separate from the rest of your work. - - Don't bother with separate commits for fixing review comments. Amend the last one, unless the review comment asks to change the `schema`-affecting work. In that case, you want to rebase interactively. - Add tests for functionality in the same commit as that functionality. ## Tests and Benchmarks