Split warnings and info annotations in API response#14327
Split warnings and info annotations in API response#14327juliusv merged 4 commits intoprometheus:mainfrom
Conversation
|
@beorn7 PTAL |
bboreham
left a comment
There was a problem hiding this comment.
This change lgtm.
The existing implementation by which errors.Is(err, PromQLInfo) is working seems a bit tricksy to me; better done via Go types.
I guess this was my idea, seduced by the convenience of using |
beorn7
left a comment
There was a problem hiding this comment.
Thank you very much. Looks good in general. I just have a number of ideas about naming. See individual comments.
|
The comment in lines 116 to 119 needs an update. (I cannot comment on unchanged lines directly, so I'm providing the link here.) |
53d1a03 to
cbb5c42
Compare
|
Making it a draft so we can wait for the frontend to update first before merging. |
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
d6f46aa to
92adad9
Compare
|
@beorn7 PTAL |
There was a problem hiding this comment.
Looks good. Thank you.
Do you think you could add a "minimal UI change" to this PR? Like rendering the infos in the same way as the warnings, just in a slightly less alarming color. That would be an MVP we could merge, and then we could ask the UI experts to make it nicer later, like including a button to hide infos or something.
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
|
@beorn7 Added the minimal UI change |
|
UI change looks good to me besides the comment on the color. |
Co-authored-by: Julius Volz <julius.volz@gmail.com> Signed-off-by: zenador <zenador@users.noreply.github.com>
Addresses #14135