Data: Add Numeric Kind constants, link to DP docs #560
Conversation
kylebrandt
requested review from
toddtreece,
ryantxu,
bohandley and
yesoreyeram
kylebrandt
added
documentation
kylebrandt
requested review from
wbrowne and
marefr
and removed request for
a team
| FrameTypeTable = "table" | ||
| ) | ||
| // --- | ||
| // Docs Note: Constants need to be on their own line for links to work with the pkgsite docs. |
There was a problem hiding this comment.
is there a way to preview the output?
There was a problem hiding this comment.
I run:
https://github.com/golang/pkgsite
# kbrandt @ kbrandt-ws in ~/go/github.com/grafana/grafana-plugin-sdk-go on git:frameTypeNumeric x [12:22:58] C:130
$ pkgsite -http localhost:9123 -gorepo .
and then my url is http://localhost:9123/github.com/grafana/grafana-plugin-sdk-go@v0.0.0/data
| const FrameTypeTimeSeriesLong FrameType = "timeseries-long" | ||
|
|
||
| // FrameTypeTimeSeriesMany is the same as "Wide" with exactly one numeric value field | ||
| // Deprecated: use FrameTypeTimeSeriesMulti instead. |
There was a problem hiding this comment.
does Deprecated need a line break above it to be recognized?
There was a problem hiding this comment.
Doesn't seem to help, but cleaner so will change anyways
There was a problem hiding this comment.
seems like it works with linting, but not pkgsite
There was a problem hiding this comment.
So it seems, need to make sure FE understands it first, will either wait a little bit after checking or move that particular one to another PR.
There was a problem hiding this comment.
Is this what you had asked me to work on @kylebrandt? Will this help with pkgsite? grafana/grafana#59070
| FrameTypeTimeSeriesMulti, | ||
| FrameTypeTimeSeriesMany, | ||
|
|
||
| FrameTypeNumericWide, | ||
| FrameTypeNumericLong, | ||
| FrameTypeNumericMulti: | ||
| return true | ||
| } |
There was a problem hiding this comment.
FrameTypeDirectoryListing, FrameTypeTable are still considered UnknownTypes? It is still unclear where and how the IsKnownType should be used.
There was a problem hiding this comment.
I'm not sure on use for IsKnownType() either...
| case p.IsTimeSeries(): | ||
| return KindTimeSeries | ||
| case p.IsNumeric(): | ||
| return KindNumeric | ||
| default: | ||
| return KindUnknown | ||
| } |
There was a problem hiding this comment.
same as previous comment. Don't we have kind for FrameTypeDirectoryListing \ FrameTypeTable?
There was a problem hiding this comment.
Good catch. I'm not sure what they should be. Going with Table, is seems we could say:
- Kind is Table
- Format is Table
- We abbreviate "TableTable" as "Table"
Could do the same for "directory-listing". I guess the assumption here is there will only ever be one format for the kind, but we could do "DirectoryListingSomething" or "TableSomething" if we have to someday. WDYT?
There was a problem hiding this comment.
I'm also perhaps unsure about "Table" itself.
For example, with a SQL query like like Select Avg(foo), city group by city, at least in SSE, it would be looking for "NumericLong", but the in the SQL query UI in grafana that would be format as "Table". And this also makes me realize the collision on the word "format" between the contract and UI.
There was a problem hiding this comment.
Moved these to #572 so can go forward with this PR for now.
What this PR does / why we need it:
Special notes for your reviewer:
Puts things in their own constant lines so the link documentation works with
pkgsite(go package documentation).Didn't do heatmap yet, that could be another PR.