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 Development Best Practices and Terraform Best Practices.

  1. Use the “Suggest” feature as much as possible. This makes it quick and easy for the contributor to accept or dismiss the recommendations.
  2. Use proper markdown in suggestions (e.g. code blocks)
  3. Always be polite and appreciative of the contributions!
  4. Use emoticons to up-vote other comments (rather than +1 comments)
  5. Use ChatOps commands like /rebuild-readme or /terraform-fmt to fix common problems
  6. Use ChatOps commands like /test all, /test bats, /test readme, /test terratest to run integration tests
  7. Recommend changes to better conform to our best-practices.
  8. 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 from README.yaml. Anything you want to update in the README must be updated in README.yaml or it will simply be overwritten. Updating the README 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.)