terraform-google-modules / terraform-google-bootstrap

Bootstraps Terraform usage and related CI/CD in a new Google Cloud organization
https://registry.terraform.io/modules/terraform-google-modules/bootstrap/google
Apache License 2.0
210 stars 145 forks source link

Extra comma looks inconsistent. #36

Closed sbadakhc closed 4 years ago

sbadakhc commented 4 years ago

I noticed an extra comma at the end of the roles declarations which doesn't exist in the api declarations.

variable "activate_apis" {
  description = "List of APIs to enable in the seed project."
  type        = list(string)

  default = [
    "serviceusage.googleapis.com",
    "servicenetworking.googleapis.com",
    "compute.googleapis.com",
    "logging.googleapis.com",
    "bigquery.googleapis.com",
    "cloudresourcemanager.googleapis.com",
    "cloudbilling.googleapis.com",
    "cloudkms.googleapis.com",
    "cloudbuild.googleapis.com",
    "iam.googleapis.com",
    "admin.googleapis.com",
    "appengine.googleapis.com",
    "storage-api.googleapis.com",
    "monitoring.googleapis.com"    <--- no comma
  ]
}

variable "sa_org_iam_permissions" {
  description = "List of permissions granted to Terraform service account across the GCP organization."
  type        = list(string)
  default = [
    "roles/billing.user",
    "roles/compute.networkAdmin",
    "roles/compute.xpnAdmin",
    "roles/iam.securityAdmin",
    "roles/iam.serviceAccountAdmin",
    "roles/logging.configWriter",
    "roles/orgpolicy.policyAdmin",
    "roles/resourcemanager.folderAdmin",
    "roles/resourcemanager.organizationViewer",    <---comma
  ]
}
bharathkkb commented 4 years ago

I personally try to use dangling commas for multiline lists for cleaner diffs if the language supports it. More details here

sbadakhc commented 4 years ago

Fair enough but lets try and be consistent.

morgante commented 4 years ago

I'm not inclined to take a strong position either way, particularly if terraform fmt doesn't.

We would accept a PR changing this but otherwise not going to worry about it.