Code review comment for lp:~hloeung/landscape-client-charm/show-charm-source-version

Revision history for this message
Haw Loeung (hloeung) wrote :

> Although I do like some useful debug info, I see a few small things:
> 1) Obvious but essential nitpick. please run `make lint`. Some lines are too
> long.

Fixed.

> 2) I see no reason to truncate revisions. This is not git and we're not saving
> saving bytes here.
> 3) The truncated version is useless as this is a bzr repo (e.g.
> <email address hidden> truncates to
> 'haw.loeu…')

It's shown in the juju status output, we usually use juju status --format=tabular so would be useful to try keep this short. I've fixed it now, it should show:

| 20200222-auxx19qq

Elsewhere, we're using this. e.g.:

| $ jsft | grep 'Ready.*source'
| apt-stresstest/0* active idle 3 52.224.81.63 Ready (source version/commit efcc988-…)
| ubuntu-repository-cache/0* active idle 0 52.146.31.10 80/tcp Ready (source version/commit 20210217-q3mg90b4)

> 4) This seems like it would be more useful in the debug logs than just in the
> ephemeral status line. What's the exact use case?

status_set() is also logged in the juju unit logs.

In our use case, we have periodic jobs to dump out juju status info. This allows us to use tools such as grep to check across multiple environments/models and find old and outdated charms to upgrade.

review: Needs Resubmitting

« Back to merge proposal