-
Notifications
You must be signed in to change notification settings - Fork 807
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
Update 101-front-door-standard-premium to use azurerm 3.67.0 #242
Update 101-front-door-standard-premium to use azurerm 3.67.0 #242
Conversation
@@ -3,7 +3,7 @@ terraform { | |||
required_providers { | |||
azurerm = { | |||
source = "hashicorp/azurerm" | |||
version = "~> 3.27.0" | |||
version = "~> 3.67.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johndowns Why are you restricting the provider version to 3.67.x instead of using "~>3.0", which uses 3.x ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originally I think we followed along with the other samples, which seem to use this approach (or did at the time, anyway).
I'd be happy to change this to ~> 3.0
if that's the standard - however, given there's been a breaking change somewhere between 3.27 and 3.67 (and hence the changes to app-service.tf in this PR), I assumed it's best to specify a version to avoid breaking changes affecting the sample.
Please let me know what you'd like me to do and I'll adjust as required.
Thanks for opening this pr @johndowns, the e2e test passed and the code LGTM. I understand your concern, |
Thanks for the explanation. As you noted, we have changed the way we restrict the version. The problem with pinning a sample to a specific version is that updating the version info could be forgotten over a long time, requiring a lot of changes to bring the code up to date. @lonegunmanb has implemented an e2e testing pipeline that runs weekly so that we can learn of breaking issues immediately and address as needed. Therefore, I would suggest that we use "~>3.0". |
Thanks @TomArcherMsft - updated and it looks like the tests pass. Please let me know if you need anything else to merge this. |
@johndowns @lonegunmanb @grayzu @stemaMSFT |
No description provided.