Skip to content

Appease TFLint#1506

Merged
sarahseewhy merged 8 commits into
mainfrom
appease_tflint
Apr 14, 2025
Merged

Appease TFLint#1506
sarahseewhy merged 8 commits into
mainfrom
appease_tflint

Conversation

@AP-Hunt
Copy link
Copy Markdown
Contributor

@AP-Hunt AP-Hunt commented Apr 8, 2025

What problem does this pull request solve?

Trello card: https://trello.com/c/G1BmcjHp/646-relax-by-appeasing-tflint

We introduced TFLint a while back, took the failures as warnings, and set ourselves a deadline to fix them. We are now fixing them!

Things to consider when reviewing

  • Ensure that you consider the wider context.
  • Does it work when run on your machine?
  • Is it clear what the code is doing?
  • Do the commit messages explain why the changes were made?
  • Are there all the unit tests needed?
  • Has all relevant documentation been updated?

Reminders

If you've made changes to the deployer role (files in modules/deployer-access):

  • Remember to run make <environment> forms/account apply on the relevant environments (dev, staging, user-research, and/or prod)
  • Check the #govuk-forms-deployment-notifications Slack channel to ensure the apply-forms-terraform-<environment> pipelines have run successfully

Copy link
Copy Markdown
Contributor

@cadmiumcat cadmiumcat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Niiice, it all looks good to me 🙌

I love the idea of setting a deadline for fixing these

AP-Hunt added 7 commits April 11, 2025 15:12
Disabled because we use a shared "inputs.tf" file and a set of ".tfvars"
filesin a lot of places and tflint is unable to tell the difference between a
variable being unused in a particular root and a variable being completely
unused.
Later commits will address the failures that cannot be automatically fixed.
Previously we pointed TFLint at the "infra/deployments/forms" directory (for
example) and used the "--recursive" flag to have it check each directory. This
was causing it to evaluate the submodules of those roots as their own roots.

Instead of doing that, we now use the existing lists of roots within the
Makefile to run TFLint against each root individually, and do away with
"--recursive".
We previously were taking  TFLint's errors as warnings, and giving ourselves
until a deadline to fix them.

We have now fixed them and can remove the deadline code.
@sarahseewhy sarahseewhy merged commit 61df5e9 into main Apr 14, 2025
2 checks passed
@sarahseewhy sarahseewhy deleted the appease_tflint branch April 14, 2025 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants