Code review comment for lp:~jose/charms/precise/owncloud/port-change+repo+ssl-support

Revision history for this message
Charles Butler (lazypower) wrote :

hey Jose,

Thanks for the continued effort on getting this merge prepared for the charm store.

I've had a chance to review, and per our feedback in IRC i see that you've added configuration options when using the deploy from source option. This will help aleviate additional stress on you as the charm maintainer when users want to run from more recent copies upstream, and the deployment routine hasn't changed.

I've taken some time to grease the wheels and deploy owncloud in it's various configurations. Ideally I'd like to see more tests against this charm with those various deployment models, such as generating SSH keys post deployment, deploying with SSH keys as a config option, and switching up the deployment model from DEBS to SOURCE, and back post deployment.

Otherwise I've run several deployments and they all came back good for me on my MAAS host and AWS.

I'm going to approve this, but caution you to break up future merges into smaller digestable chunks as this was a fairly large review that ultimately delayed the merge from happening as quickly as we would like.

Thanks again for such dedication and great work.

+1 LGTM

review: Approve

« Back to merge proposal