-
Notifications
You must be signed in to change notification settings - Fork 728
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 spec for resource conditions #966
Conversation
Codecov Report
@@ Coverage Diff @@
## main #966 +/- ##
=======================================
Coverage 94.33% 94.33%
=======================================
Files 316 316
Lines 14771 14771
Branches 12 12
=======================================
Hits 13934 13934
Misses 837 837
Flags with carried forward coverage won't be shown. Click here to find out more. |
``` | ||
param deployZone bool | ||
|
||
resource dnsZone 'Microsoft.Network/dnszones@2018-05-01' when (deployZone) = { | ||
resource dnsZone 'Microsoft.Network/dnszones@2018-05-01' = if (deployZone) { |
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.
We never clarified it in the previous version of the spec, but are we making parentheses required?
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.
Good question...I don't think they are needed. We don't uses parentheses for list comprehensions, so it might be better to keep them consistent. What do you think?
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.
Personally I vote to require them - just because I feel like it'll seem more familiar to anyone used to C-style languages. We could technically relax this in the future if there's a good case for it. Going from non-required -> required would be a breaking change.
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.
Going from non-required -> required would be a breaking change.
Good point. Let's keep the parentheses then.
No description provided.