Code review comment for lp:~corey.bryant/charms/trusty/keystone/git

Revision history for this message
Billy Olsen (billy-olsen) wrote :

A few general comments below and in-line (some really belong on the charm-helpers merge and I acknowledge that, but it was seen while reviewing this change). Mainly the test portion needs resolution...

- Needed to do an additional make sync to get unit_tests to run & successful deployment
- Needed to mock out git_install_requested, though it doesn't appear to have broken any tests when locally mocked, but no new coverage either.

Bigger comments which are outside the scope of this review (but I want to capture them)

- It'd be kind of nice if there was a way to specify the equivalent GIT_BASE variable as in devstack's stackrc (as I had to change to https:// instead of git:// to run in our stack).

- hacluster will work with the keystone service since its only carrying the vip across the api endpoints in an active-active. However, this won't be the case for all services... but being able to run ha against tree is quite nice.

review: Needs Fixing

« Back to merge proposal