-
Notifications
You must be signed in to change notification settings - Fork 212
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
Improve the ruleset_rule
time_frame
documentation
#340
Improve the ruleset_rule
time_frame
documentation
#340
Conversation
1a54528
to
75f4378
Compare
Specifically, the previous description of how to choose a `start_time` value was misleading & did not explain how picking a correct value also involves the specified `timezone` Also update the code example to use the `time_static` resource.
75f4378
to
49c5f46
Compare
Co-authored-by: Tyurina <tyurina@pagerduty.com>
187850e
to
9321231
Compare
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.
Overall, looks great! Please just update the variables to use the current syntax. Thanks!
Ah! Good catch, @stmcallister ! I've removed those 3 cases of "interpolation-only expressions" in e3f0ec6 and tested locally to confirm that the syntax still works as expected 👍 |
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.
👍 🎉 Awesome! Thank you! LGTM.
Thanks so much for getting this merged! Will the documentation changes to live at https://registry.terraform.io/providers/PagerDuty/pagerduty/latest/docs/resources/ruleset_rule when we (i.e. you) release a new version of the terraform provider? |
Specifically, the previous description of how to choose a
start_time
value was misleading & did not explain how picking a correct value also involves the specifiedtimezone
Also update the code example to demonstrate how to use the
time_static
resource for slightly more human-readable configuration. Best I could tell there is not a way to use other built-in time functions likeformatdate
to create a unix epoch timestamp, which is why I picked thetime_static
resource.A future improvement to the provider might include allowing users to specify
start_time
and other unix timestamp attributes using RFC 3339 "Date and Time format" syntax, but that improvement is out of the scope of this PR.