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

Update table widget definition and request #686

Merged
merged 9 commits into from
Oct 16, 2020
Merged

Conversation

sighrobot
Copy link
Contributor

@sighrobot sighrobot commented Oct 6, 2020

Adds new properties to the table widget definition and request

@sighrobot sighrobot requested a review from a team as a code owner October 6, 2020 18:03
@sighrobot sighrobot self-assigned this Oct 6, 2020
@therve
Copy link
Contributor

therve commented Oct 6, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@nmuesch nmuesch left a comment

Choose a reason for hiding this comment

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

Quick question on the type

@@ -3297,6 +3297,10 @@ func getQueryTableDefinitionSchema() map[string]*schema.Schema {
Schema: getWidgetTimeSchema(),
},
},
"has_search_bar": {
Type: schema.TypeString,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be a bool?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this is a string enum ("always", "never", or "auto") – is there a way to specify an enum?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh ok thanks. You could do something like set a ValidateFunc with stringInSlice - https://github.com/hashicorp/terraform/blob/master/helper/validation/validation.go#L32 but I think its OK to just document the allowed fields too.

datadog/resource_datadog_dashboard.go Outdated Show resolved Hide resolved
@sighrobot sighrobot force-pushed the abe/table-widget-updates branch from 2468a44 to 4eb75b0 Compare October 8, 2020 14:56
@sighrobot sighrobot requested a review from a team as a code owner October 8, 2020 14:56
@sighrobot sighrobot changed the title [VIZZ-954] Update table widget definition and request Update table widget definition and request Oct 8, 2020
Copy link
Contributor

@therve therve left a comment

Choose a reason for hiding this comment

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

That looks good, can you tweak the documentation to mention the new field? Thanks.

@sighrobot sighrobot force-pushed the abe/table-widget-updates branch from c4ca78b to 57300d8 Compare October 14, 2020 23:35
@sighrobot
Copy link
Contributor Author

@therve this is passing now and docs updated. We should probably wait to merge until the upstream correction to the schema (for cell_display_mode) is merged!

@sighrobot sighrobot force-pushed the abe/table-widget-updates branch from 80f49dd to c133987 Compare October 15, 2020 18:24
@therve
Copy link
Contributor

therve commented Oct 16, 2020

/azp run

@therve
Copy link
Contributor

therve commented Oct 16, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@therve therve merged commit 180aa61 into master Oct 16, 2020
@therve therve deleted the abe/table-widget-updates branch October 16, 2020 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants