Skip to content

Conversation

@tomwilkie
Copy link
Contributor

Previously, this would happen automatically as all mixins were merged into one. Now we keep mixins in different namespaces, this needs to be done by out grafana jsonnet code.

Opinions are:
// - Dashboard UIDs should be the md5 hash of their filename.
// - Timezone should be default (ie local).
// - Tooltip should only show a single value.

Signed-off-by: Tom Wilkie [email protected]

…shboards.

Previously, this would happen automatically as all mixins were merged into one.  Now we keep mixins in different namespaces, this needs to be done by out grafana jsonnet code.

Opinions are:
  // - Dashboard UIDs should be the md5 hash of their filename.
  // - Timezone should be default (ie local).
  // - Tooltip should only show a single value.

Signed-off-by: Tom Wilkie <[email protected]>
@tomwilkie tomwilkie force-pushed the dashboard-opinions branch from 48d9a2f to 0626512 Compare April 8, 2020 09:31
Copy link
Member

@tombrk tombrk left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

local mixinProto = {
grafanaDashboards+:: {},
} + {
local grafanaDashboards = super.grafanaDashboards,
Copy link
Member

Choose a reason for hiding this comment

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

just to clarify, super here is the already merged $.mixins[mixinName] + { grafanaDashboards+:: {} }?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, thats just there so we can reliably reference super.grafanaDashboards (you can do std.hasField(super, ...).

Comment on lines 18 to 20
// - Dashboard UIDs should be the md5 hash of their filename.
// - Timezone should be "default" (ie local).
// - Tooltip should only show a single value.
Copy link
Member

Choose a reason for hiding this comment

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

should sounds like we do validation here – but we actually just force this behaviour

Signed-off-by: Tom Wilkie <[email protected]>
@tomwilkie tomwilkie merged commit 5490026 into master Apr 8, 2020
@tomwilkie tomwilkie deleted the dashboard-opinions branch April 8, 2020 13:01
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.

2 participants