forked from rook/rook
docs: document the CI checks that gate a pull request
The development flow guide described running unit tests and `make lint`, but several other checks also gate every pull request and were documented nowhere: `make golangci-lint`, `make codegen`, `make crds`, and commitlint. A contributor had no way to learn about them short of pushing and reading a red CI run. Add a table of the gating checks and the command that runs each one locally, and document the parts that are easy to get wrong: - Any change under `pkg/apis` requires regenerating the client, deepcopy functions, CRD manifests, and CRD reference docs, and the regenerated files belong in the same commit. This includes editing a godoc comment, since those comments are copied verbatim into the CRD description fields. - Commitlint infers a trailer from the shape of a line, so a body line that begins with a word followed by a colon is read as one, and the commit is rejected with `footer-leading-blank`. - The commit prefix must come from the type list in `.commitlintrc.json`. Also note that `make test` only covers `cmd/` and `pkg/`, so test files under `tests/framework/` are compiled but never run by the unit test CI. Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
This commit is contained in:
@@ -218,6 +218,30 @@ as much as possible before pushing
|
||||
(a behavior that is not required but highly encouraged),
|
||||
the unit tests and various linters can easily be run locally.
|
||||
|
||||
### Checks that gate a pull request
|
||||
|
||||
Each of the following checks runs against every PR and must pass before it can be merged.
|
||||
Every one of them can be run locally, and doing so before pushing saves a full CI round trip:
|
||||
|
||||
| Check | Run it locally | Applies to |
|
||||
| ----- | -------------- | ---------- |
|
||||
| Unit tests | `make test` | Go changes under `cmd/` and `pkg/` |
|
||||
| Go linters | `make golangci-lint` | any Go change |
|
||||
| Generated client and deepcopy code | `make codegen` | any change under `pkg/apis` |
|
||||
| CRD manifests and reference docs | `make crds` | any change under `pkg/apis` |
|
||||
| Commit message format | `make lint.commits` | every commit |
|
||||
| Markdown formatting | `make lint.markdown` | any change under `Documentation/` |
|
||||
|
||||
!!! tip
|
||||
Build and test through `make` rather than invoking `go` directly. The `make` targets set the
|
||||
build flags and tags that Rook needs, so an ad-hoc `go build` or `go test` may not match what CI
|
||||
runs. Run `make help` to see all targets.
|
||||
|
||||
!!! note
|
||||
`make test` only runs the unit tests under `cmd/` and `pkg/` (the `GO_SUBDIRS`). Test files
|
||||
elsewhere, such as under `tests/framework/`, are compiled by the linters but are never run by
|
||||
the unit test CI.
|
||||
|
||||
## Unit Tests
|
||||
|
||||
From the root of your local Rook repo execute the following to run all of the unit tests:
|
||||
@@ -267,6 +291,30 @@ Refer to the respective project website for more installation instructions.
|
||||
!!! tip
|
||||
Run `make help` to see a list of all possible make targets
|
||||
|
||||
## Generated Code and CRDs
|
||||
|
||||
Several checked-in files are generated from the API types under `pkg/apis` and must be regenerated
|
||||
whenever those types change:
|
||||
|
||||
```console
|
||||
# Regenerates the deepcopy functions and the typed client under pkg/client
|
||||
make codegen
|
||||
|
||||
# Regenerates the CRD manifests and the CRD reference documentation
|
||||
make crds
|
||||
```
|
||||
|
||||
Commit the regenerated files together with the change that required them, in the same commit. CI
|
||||
fails if regenerating produces any diff.
|
||||
|
||||
A change to `pkg/apis` means more than adding or removing a field. Editing the godoc comment on an
|
||||
exported type or field also requires regeneration, because those comments are copied verbatim into
|
||||
the `description` fields of the CRDs. Reword a comment and the following files change:
|
||||
|
||||
* `deploy/examples/crds.yaml`
|
||||
* `deploy/charts/rook-ceph/templates/resources.yaml`
|
||||
* `Documentation/CRDs/specification.md`
|
||||
|
||||
## Integration Tests
|
||||
|
||||
Rook's upstream continuous integration (CI) tests will run integration tests against your changes
|
||||
@@ -302,6 +350,28 @@ Then I'm explaining how I fixed it.
|
||||
Signed-off-by: FirstName LastName <email address>
|
||||
```
|
||||
|
||||
The commit prefix must be one of the types listed under `rules.type-enum` in
|
||||
[`.commitlintrc.json`](https://github.com/rook/rook/blob/master/.commitlintrc.json). Only those
|
||||
types are accepted, so pick the closest match rather than inventing one after the package being
|
||||
changed.
|
||||
|
||||
Separate any trailer from the body with a blank line, or commitlint reports
|
||||
`footer-leading-blank`. Note that commitlint decides what a trailer is by looking at the text: a
|
||||
line that begins with a word followed by a colon, or one that closes an issue, is treated as a
|
||||
trailer even when it was meant as prose. Wrapping a sentence so that a line happens to begin with
|
||||
`anywhere:` is enough to trigger this. Rewrap the line to avoid it. Mentioning an issue in the
|
||||
middle of a sentence is fine.
|
||||
|
||||
Check the commit messages on your branch before pushing:
|
||||
|
||||
```console
|
||||
make lint.commits
|
||||
```
|
||||
|
||||
This runs the same commitlint that CI runs, over your branch's commits. It compares against
|
||||
`origin/master` by default; if your Rook upstream is a different remote, point it there with
|
||||
`make lint.commits COMMITLINT_BASE=upstream/master`.
|
||||
|
||||
### Commit History
|
||||
|
||||
To prepare your branch to open a PR, the minimal number of logical commits is preferred to maintain
|
||||
|
||||
Reference in New Issue
Block a user