Code review comment for ~vultaire/charm-sysconfig:lp1841809

Revision history for this message
Alvaro Uria (aluria) wrote :

Hey Paul. I've added a comment inline, but I will repeat it here.
* checksum verification can be achieved via any_file_changed [1], helper from the reactive framework, A list of file(s) needs to be passed.
* "render" already checks if new content is going to be written, or it will skip the write
* As a suggestion of a diff approach, set_flag("systemd-resolved.restart") could always be set after the update and remove functions are called (maybe even merging the action into a single method?) and any_file_changed could be evaluated in the decorated function (within the reactive script), to see if a service restart is needed or not (+ clear the flag).

And think the above will make more sense in the comment inline.

Other than this, the change (and tests) look great.

1. https://github.com/juju-solutions/charms.reactive/blob/master/charms/reactive/helpers.py#L85

review: Needs Fixing

« Back to merge proposal