Code Review Guidelines
Here are some of our tips for conducting Code Reviews the SweetOps way. If you haven't already, become familiar with our Best Practices and Terraform Best Practices.
- Use the "Suggest" feature as much as possible. This makes it quick and easy for the contributor to accept or dismiss the recommendations.
- Use proper markdown in suggestions (e.g. code blocks)
- Always be polite and appreciative of the contributions!
- Use emoticons to up-vote other comments (rather than
+1
comments) - Use ChatOps command
/terratest
to run integration tests - Recommend changes to better conform to our best-practices
- Quote comments you're replying to make your responses more clear
Specifics for Terraform Modules
We use automated testing to enforce certain standards for our Terraform modules. Currently these are run via GitHub Actions, and you can look at the logs of failing tests by clicking the Details
link in the PR status list. Here is a partial list of rules that are enforced:
- All modules referenced must be pinned to an exact, numbered version. Cannot be
master
or a range like>= 0.9.0
- All providers must have version pinning of the form
>=
(can be>= x.x
or>= x.x.x
). More restrictive pinning is not allowed. - All modules that no longer support Terraform versions older than 0.12.26 must be upgraded to refer to providers using Terraform Registry format (explicit
source
field). - All modules must have their
README
updated to the current standard. Note:README.md
is generated by tooling fromREADME.yaml
. Anything you want to update in theREADME
must be updated inREADME.yaml
or it will simply be overwritten. Updating theREADME
usually requires nothing more than regenerating it. - All modules must comply exactly with Terraform formatting standards used by
terraform fmt
We have tooling to help with some of this. Before opening a PR, but after making all your changes, run
make pr/auto-format
in the root directory of the repository. That will format your Terraform code and rebuild the README. (If you have done that and the tests still complain about a bad README
, it is possible you have cached an old version of the builder Docker image. Try updating it with make init && make builder/pull
and run make pr/auto-format
again.)