Skip to content

Commit

Permalink
build: Remove our CircleCI configuration for PR checks
Browse files Browse the repository at this point in the history
We will henceforth use the "checks.yml" GitHub Actions workflow instead of
CircleCI, because we're standardizing on using GitHub Actions for all of
our automation in this repository so that everything is in a consistent
language and we have as few external dependencies as possible.

The checks.yml workflow alone does not actually replace everything this
CircleCI configuration did. Reworking things for GitHub Actions was a good
opportunity to revisit the cost/benefit of the various steps here and my
conclusions were:
- Unit tests and consistency checks give the best signal about the
  correctness of new code, with broad coverage over all of our packages.
  These are the most important things we want to run before reviewing a
  pull request, although our unit test run is currently relatively slow
  and would probably be worth optimizing in future commits.
- Our existing build.yml workflow already runs the E2E tests across
  various platforms and so I considered removing those but elected to keep
  the same single-platform (Linux) E2E test run in the pre-review checks
  because in practice those tests are typically faster than the full
  unit test run anyway and so they don't delay a green check result and
  can serve as a reasonable proxy for whether the cross-platform E2E tests
  will all succeed when we eventually check in build.yml, after merge.
- We've long had a special exception to our usual rule of not running
  acceptance tests in CI specifically for the Consul backend. In practice
  the Consul backend is essentially "done" and doesn't change much, so
  I don't think the cost of installing and launching Consul just to test
  that one backend has sufficient benefit to preserve. Our unit tests do
  still exercise all of the generic backend machinery via the inmem and
  local backends, and in the event that someone does make changes to the
  Consul backend they can still run the acceptance tests locally as we'd
  expect for a change to any other backend.
- We previously included jobs to run "go build" across various different
  platforms. Although that can occasionally help catch platform-specific
  issues, most code in Terraform is platform-agnostic and so it's rare
  to encounter single-platform build errors. These jobs were typically
  the long pole for completion of the CI checks before and so I've removed
  them here in favor of relying on similar checks already happening inside
  the build.yml workflow, which runs only after a PR is merged. This does
  increase the risk of a platform-specific error landing in a release
  branch before we catch it, but since platform-specific problems are
  rare this feels like a reasonable tradeoff. Anyone working on
  explicitly-platform-specific code in Terraform should typically test
  locally on the relevant platform anyway, and so catching these with our
  build step is a last gate just to make sure mistakes don't end up in
  production releases.
  • Loading branch information
apparentlymart committed Apr 4, 2022
1 parent bbf540e commit 1e56e1f
Showing 1 changed file with 0 additions and 200 deletions.
200 changes: 0 additions & 200 deletions .circleci/config.yml

This file was deleted.

0 comments on commit 1e56e1f

Please sign in to comment.