Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linting, security scans and docs 🥇 #1

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

alismx
Copy link
Collaborator

@alismx alismx commented Nov 6, 2024

DEVOPS PULL REQUEST

Related Issue

Changes Proposed

Additional Information

  • tflint will not be required for approvals until we get used to the workflow and demonstrate that it doesn't impede our work

… unuses variables, and pinning provider versions
@alismx alismx changed the title add workflows for linting, resolve several linting issues by removing… Linting, security scans and docs 🥇 Nov 7, 2024
@alismx alismx marked this pull request as ready for review November 7, 2024 18:01
registry_username = data.aws_ecr_authorization_token.this.user_name
registry_password = data.aws_ecr_authorization_token.this.password
service_data = {
service_data = var.service_data == {} ? {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Small item, non-blocking: would it be more legible for us to use the coalesce function here, so the last part of the ternary isn't dangling off the end of the block a few dozen lines down?

More info here: https://developer.hashicorp.com/terraform/language/functions/coalesce

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hey! I looked at coalesce, which seems like a good win for readability. 🔺

I also took a peak at merge, I could see using it to allow for more refined control of the service_data object without needing to pass in the entire blob. ( like maybe I just want the orchestration service to have more memory) That said, it will not work for today, so I'll file that away in the maybe someday column.

ecr-viewer = {
short_name = "ecrv",
fargate_cpu = 1024,
fargate_memory = 2048,
min_capacity = 1
max_capacity = 5
min_capacity = 1,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, no. This is going to be one of these stylistic areas that gives me fits. Is this JSON or YAML here? I've got a similar structure in dibbs-azure for the block definitions that uses no commas whatsoever. I'm in the camp that they clutter things unnecessarily unless they are explicitly needed by the language spec.

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.

Automatically label new issues Add tflinting Update README Enable Trivy security scanning
2 participants