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

fix(compute-mig): add correct type optionality for metrics in autosca… #1668

Merged

Conversation

NotArpit
Copy link
Contributor

@NotArpit NotArpit commented Sep 12, 2023

Description

In the //modules/compute-mig module there is a mismatch between the optionality of types and what the upstream terraform docs mention for autoscaler_config.metrics.(type|target_value). It seems only the autoscaler_config.metrics.name type is required.

Notes

I've ran tools/tfdoc.py but linting seems to be failing, can maintainers PTAL?

Checklist

I applicable, I acknowledge that I have:

  • Read the contributing guide
  • Ran terraform fmt on all modified files
  • Regenerated the relevant README.md files using tools/tfdoc.py
  • Made sure all relevant tests pass

@ludoo
Copy link
Collaborator

ludoo commented Sep 12, 2023

Pass the --files otpion to tfdoc. We should force it in the tfdoc tag itself, sorry for that.

@ludoo
Copy link
Collaborator

ludoo commented Sep 12, 2023

Well no it's the opposite: don't enable files in tfdoc :)

@ludoo ludoo enabled auto-merge (squash) September 12, 2023 08:29
auto-merge was automatically disabled September 12, 2023 11:08

Head branch was pushed to by a user without write access

@NotArpit
Copy link
Contributor Author

@ludoo all done :)

@ludoo ludoo merged commit b512650 into GoogleCloudPlatform:master Sep 12, 2023
9 checks passed
@ludoo
Copy link
Collaborator

ludoo commented Sep 12, 2023

Thanks a lot for this PR!

@NotArpit NotArpit deleted the arpit-compute-mig-metrics-type-fix branch September 12, 2023 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants