Skip to content

Commit

Permalink
docs: update PR template (#4949)
Browse files Browse the repository at this point in the history
* docs: update PR template
* docs: consider breaking changes against `main`
* docs: add review etiquette section to CONTRIBUTING.md
* docs: add note on overwriting commits
---------
Signed-off-by: Nurzhan Sakén <[email protected]>
  • Loading branch information
nxsaken authored Aug 14, 2024
1 parent edf2311 commit 5cc90fe
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 24 deletions.
41 changes: 22 additions & 19 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -1,33 +1,36 @@
## Description
<!-- Note: replace the instructions with your text -->

<!-- Just describe what you did. -->
## Context

<!-- Skip if the title of the PR is self-explanatory -->
- 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

<!-- Duplicate the main issue and add additional issues closed by this PR. -->
- Describe the approach taken to achieve the objective / resolve the issue.

Closes #{issue_number} <!-- Replace with an actual number, -->
### Migration Guide (optional)

<!-- Link if e.g. JIRA issue or from another repository -->
- 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
---

<!-- EXAMPLE: users can't revoke their own right to revoke rights -->
### 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.

<!-- HINT: Add more points to checklist for large draft PRs-->
<!-- Add more items if needed -->

<!-- USEFUL LINKS
- https://www.secondstate.io/articles/dco
- https://discord.gg/hyperledger (please ask us any questions)
- https://t.me/hyperledgeriroha (if you prefer telegram)
-->
- Commit sign-off: https://www.secondstate.io/articles/dco
- Telegram: https://t.me/hyperledgeriroha
- Discord: https://discord.com/channels/905194001349627914/905205848547155968
-->
15 changes: 10 additions & 5 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 5cc90fe

Please sign in to comment.