Code review comment for lp:~adam-collard/charms/precise/landscape-client/upgrade-charm-hook

Revision history for this message
Adam Collard (adam-collard) wrote :

> Looks good, but I think we should address [1] and [2].
>
> [1]
>
> +def upgrade_charm():
>
> Since we might need to do other things in this hook in the future, it'd be
> better to keep it lean and extract the logic into standalone (testable)
> functions. E.g.:
>
> def upgrade_charm():
> client_config = get_client_config()
> migrate_old_juju_info(client_config) # passing in client_config should
> make this function fully testable
> # future upgrade steps will go here

Done.

>
> [2]
>
> It'd be good to test this logic.

Added tests, and a Makefile to go along with them :)

« Back to merge proposal