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

Add Datadog dashboard SLO widget support #355

Merged
merged 5 commits into from
Nov 27, 2019

Conversation

mbarrien
Copy link
Contributor

@mbarrien mbarrien commented Oct 31, 2019

Adds support for the SLO widget in a Datadog dashboard. Can't find an official source of text on Datadog's API website, so created my own documentation not from official sources.

@mbarrien
Copy link
Contributor Author

Ping @nmuesch ?

@KeisukeYamashita
Copy link

👍

I want SLO widget too in this terraform provider

Copy link
Contributor

@platinummonkey platinummonkey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! thanks for doing this ❤️

@nmuesch this looks good to me for the widget 👍

@mbarrien
Copy link
Contributor Author

Rebased against master, which now already has a version of go-datadog-api with the necessary APIs, so no more updates to go.mod in this PR. Please review and merge!

Copy link
Contributor

@bkabrda bkabrda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, the code looks very good. I made a couple minor comments on the tests to address and would also like to ask you to address @platinummonkey comments. When that's done, we can go ahead and merge.

datadog/resource_datadog_dashboard_test.go Outdated Show resolved Hide resolved
datadog/resource_datadog_dashboard_test.go Outdated Show resolved Hide resolved
website/docs/r/dashboard.html.markdown Show resolved Hide resolved
@KeisukeYamashita
Copy link

@bkabrda We fix according to your comments. Please review and merge:)

@bkabrda
Copy link
Contributor

bkabrda commented Nov 27, 2019

Code looks great, all comments have been addressed and tests are passing. LGTM, merging. Thank you very much for the contribution!

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.

4 participants