-
Notifications
You must be signed in to change notification settings - Fork 9
feat: add colorMapping property type to the swagger definition #248
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
Conversation
| colorMapping: | ||
| description: An object that contains information about the color mapping | ||
| $ref: "./ColorMapping.yml" |
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.
These changes are mine.
| type: object | ||
| description: maps series id to the color index that is used to index into the colors array | ||
| additionalProperties: | ||
| type: int | ||
| example: | ||
| series_id_1: 0 | ||
| series_id_2: 1 |
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.
And these. Rest are generated.
hoorayimhelping
left a comment
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.
this is solid and will work just fine, but i gave a couple suggestions to make the documentation a little bit richer. let me know what you think
contracts/common.yml
Outdated
| - explicit | ||
| ColorMapping: | ||
| type: object | ||
| description: maps series id to the color index that is used to index into the colors array |
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.
I think this is a bit of an obvious comment. I think a better description would explain what a color mapping is so that someone reading this definition who wasn't familiar with color mapping would understand what it's for.
A color mapping is an object that maps time series data to a UI color scheme to allow the UI to render graphs consistent colors across reloads.
| type: int | ||
| example: | ||
| series_id_1: 0 | ||
| series_id_2: 1 |
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.
this is a decent example, but it might be more illustrative if the series ids were more like how they'll be in real life using a common example that many people in the company would be familar with:
measurement_birdmigration_europe: 0
that isn't a very good example, but I'm sure you can find a more common example using sample data or something like that?
hoorayimhelping
left a comment
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 for updating those examples! solid work 👍
Closes: #243
Depends on: https://github.com/influxdata/idpe/pull/12366
This PR adds the colorMapping property to the swagger definition. I ran
make generateto update the other files needed.