refactor: remove disablemetrics for enablemetrics #554
Conversation
Signed-off-by: nerdeveloper <odirionye@gmail.com>
Signed-off-by: nerdeveloper <odirionye@gmail.com>
|
BTW , what issue the PR points to ? |
|
Just a note, we’ll also want to reflect this change in the helm chart and the docs after it’s released. @nerdeveloper let us know if you are up for that! Also we’ll want to wait for |
|
Sure, I am definitely up for that! I can see on the docs: https://chartmuseum.com/docs/#prometheus-metrics Also, I will refactor it and wait till |
Signed-off-by: nerdeveloper <odirionye@gmail.com>
|
Agree with @cbuto's idea , and this issue should be related to the charts and doc PR . |
jdolitsky
left a comment
There was a problem hiding this comment.
Thanks for tackling this.
Can you also modify the README section to describe the new behavior?
https://github.com/helm/chartmuseum#prometheus-metrics
Maybe even mention the now deprecated old flag in versions v0.14.0 and prior
pkg/config/vars.go
Outdated
| CLIFlag: cli.BoolFlag{ | ||
| Name: "disable-metrics", | ||
| Usage: "disable Prometheus metrics", | ||
| Usage: "(deprecated)disable Prometheus metrics", |
There was a problem hiding this comment.
can we add a space between (deprecated) and disable Prometheus metrics ?
There was a problem hiding this comment.
Sure, I will into this today.
Definitely |
Signed-off-by: nerdeveloper <odirionye@gmail.com>
Signed-off-by: nerdeveloper <odirionye@gmail.com>
|
I have update chart museum Helm Chart for the Prometheus config here: chartmuseum/charts#38 |
|
OMG! Yes, it has been merged! I am so happy. thank you, team. |
Signed-off-by: nerdeveloper odirionye@gmail.com