-
Notifications
You must be signed in to change notification settings - Fork 156
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
[dashboards] Support getting dashboards by string id #225
[dashboards] Support getting dashboards by string id #225
Conversation
helpers.go
Outdated
@@ -79,3 +83,18 @@ func GetPrecision(v *PrecisionT) (PrecisionT, bool) { | |||
|
|||
return PrecisionT(""), false | |||
} | |||
|
|||
// GetStringId is a helper routine that allows screenboards and timebaords to be retrieved |
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.
Small typo: timebaords
-> timeboards
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.
Thanks!
integration/dashboards_test.go
Outdated
if err != nil { | ||
t.Fatalf("Retrieving a dashboard failed when it shouldn't. (%s)", err) | ||
} | ||
assertDashboardEquals(t, actual, 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.
We could also test that Id
and NewId
are the same (between the 2 objects returned by CreateDashboard
).
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.
Left minor nits, otherwise this changes look good to me
LGTM 👍. @enbashi are you ok with me squashing all the commits when merging? |
Cool, thank you very much! Merging. |
Add the ability to use the new dashboard string ID format in
GetScreenboard()
andGetDashboard()
.Used in https://github.com/terraform-providers/terraform-provider-datadog/pull/165