Juju Charms Collection
nova-cloud-controller package

Merge lp://qastaging/~corey.bryant/charms/precise/nova-cloud-controller/step-migration into lp://qastaging/~openstack-charmers/charms/precise/nova-cloud-controller/icehouse

  1. Precise Pangolin (12.04)
  2. step-migration
  3. Merge into icehouse
Proposed by Corey Bryant
Status: Merged
Merged at revision: 102
Proposed branch: lp://qastaging/~corey.bryant/charms/precise/nova-cloud-controller/step-migration
Merge into: lp://qastaging/~openstack-charmers/charms/precise/nova-cloud-controller/icehouse
Diff against target: 60 lines (+38/-5)
1 file modified
hooks/nova_cc_utils.py (+38/-5)
To merge this branch: bzr merge lp://qastaging/~corey.bryant/charms/precise/nova-cloud-controller/step-migration
Reviewer Review Type Date Requested Status
James Page Needs Fixing
Review via email: mp+214590@code.qastaging.launchpad.net
To post a comment you must log in.
Revision history for this message
James Page (james-page) wrote :

Comments:

1) + return ''

return None would be nicer - passing empty strings always feels bad to me - to be explicit the check of the return value should be in this case:

  if step_src is not None:

2) Don't return in the stepped upgrade.

do_openstack_upgrade is currently returning on the interim step as well; not good :-).

review: Needs Fixing
Revision history for this message
Corey Bryant (corey.bryant) wrote :

> Comments:
>
> 1) + return ''
>
> return None would be nicer - passing empty strings always feels bad to me - to
> be explicit the check of the return value should be in this case:
>
> if step_src is not None:
>

Sounds good.

> 2) Don't return in the stepped upgrade.
>
> do_openstack_upgrade is currently returning on the interim step as well; not
> good :-).

Oops, yeah that's a mistake.

lp://qastaging/~corey.bryant/charms/precise/nova-cloud-controller/step-migration updated
102. By Corey Bryant

Enable upgrade from grizzly to icehouse by stepping to havana

Revision history for this message
Corey Bryant (corey.bryant) wrote :

Fixes have been added in response to James' comments. Thanks for reviewing!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: