search

juju / charm-tools

Tools for charm authors and maintainers
Other
42 stars 64 forks source link

`charm proof` should at least warn if metrics are defined but not collected #314

Open cmars opened 7 years ago

cmars commented 7 years ago

If a charm collects metrics (has a metrics.yaml file) but does not actually collect them with a collect-metrics hook (no hooks/collect-metrics), charm proof should at least provide a warning.

Note that the hook could be a bespoke hook written by the charm author, or it could be added by a layer like layer:metrics.

mbruzek commented 7 years ago

Just a reminder, a Warning from charm proof would prevent a charm from going into the store. I tend to agree with @cmars in this case if you are specifying metrics but do not have the hooks that seems wrong, I just want to make it clear that would fail a review where an Info message would not.

cmars commented 7 years ago

a Warning from charm proof would prevent a charm from going into the store

I've been able to push charms with warnings to jujucharms. I don't think we'd want to prevent that on a warning -- some charms are in development, and some warnings are to be taken as a matter of opinion (in my opinion).

Now, if you mean that a charm would not pass the standard charm review process with such a warning, that is ok.

mbruzek commented 7 years ago

That is what I meant. charm promulgate will not allow us to push a charm with warnings or errors.

marcoceppi commented 7 years ago

This seems reasonable. I'll push a PR up shortly