From 20c367c3cb3becf0adb9246eca0dbba221a77cc7 Mon Sep 17 00:00:00 2001 From: Daniil Fedotov Date: Fri, 10 May 2024 12:14:27 -0400 Subject: [PATCH] docs: add code review guide (#2772) --- CODE_REVIEW.md | 73 +++++++++++++++++++++++++++++++++++++++++++++++++ CONTRIBUTING.md | 10 +++++++ 2 files changed, 83 insertions(+) create mode 100644 CODE_REVIEW.md diff --git a/CODE_REVIEW.md b/CODE_REVIEW.md new file mode 100644 index 0000000000..d11cac9f8e --- /dev/null +++ b/CODE_REVIEW.md @@ -0,0 +1,73 @@ +# Code review requiements + +This document describe responsibilities of code reviewers when reviewing PRs. + +Pull request process is described in [contributing guide](./CONTRIBUTING.md#submitting-pull-requests) + +## Base checklist + +- All automated test steps pass (e.g. tests, lints, build) +- PR title follows [commit conventions](CONTRIBUTING.md#commit-conventions) + - If PR format is different, reviewer should change it to follow the conventions +- PR has a description with reasoning and change overview + - If description is missing but clear for reviewer, reviewer may add a comment or edit description +- New feature or fix has tests proving it works + - Reviewer should request changes from contributor to add tests +- If PR changes documented behaviour, there should be update to `/docs` or inline comments + - Reviewer should request changes from contributor to update docs +- If PR introduces breaking changes, fixes a bug or adds a new feature, there should be a [release note](#release-notes) + - Reviewer may request changes from the contributor to add a release note + - Reviewer may add a release note by themself in order to unblock the merge process + +## Requesting changes + +It's recommended to request changes by submitting `comment` type reviews. +`Request changes` type review would block merging until requester approves the +changes, this can slow down the process if there are multiple reviewers. + +## Approving and merging + +We use `kueue` bot to merge approved PRs. + +If PR is approved, all checks are passing and it has the `kueue` label, it will +be automatically squashed and merged. + +For PRs from Kanister developers, the committer should add the `kueue` label after +PR was approved. + +For PRs from community, the reviewer should add the `kueue` label. + +## Release notes + +Kanister is using the [reno](https://docs.openstack.org/reno/latest/) tool to generate changelogs from release note files. + +To add release note one could run: + +``` +make reno-new note= +``` + +Note name should be a short description of a change. + +File format is described in [reno docs](https://docs.openstack.org/reno/latest/user/usage.html#editing-a-release-note) + +Typical examples would be: + +``` +--- +features: + - Added new functionality doing X +``` + +Or: + +``` +--- +fixes: + - Fixed bug with pod output format +upgrade: + - Make sure custom blueprints follow pod output format spec +``` + +See [release notes](./releasenotes/README.md) for more info. + diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 80e7b321d2..67f8e6bce7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -95,6 +95,16 @@ We are using squash and merge approach to PRs which means that commit descriptio It's recommended to use conventional commits when strarting a PR, but follow-up commits in the PR don't have to follow the convention. +### Release notes + +If submitted change fixes a bug, introduces a new feature or breaking change, contributor should add a release note. +Kanister is using the [reno](https://docs.openstack.org/reno/latest/) tool to track release notes. + +Release note can be added with `make reno-new note=` command, which will create a note file. +Contributor should edit and commit the note file. + +See [release notes](./releasenotes/README.md) for more info. + ### Submitting Pull Requests **PR titles should be in following format:**