Comment 3 for bug 1065666

Revision history for this message
Robert Ayres (robert-ayres) wrote :

I've performed a review of your changes, please see the following bugs/comments.

Bugs
*metadata.yaml, missing maintainer field - I suspect you've erroneously added this to your README instead. (See https://juju.ubuntu.com/docs/write-charm.html if you need an example).

*hooks/install, vdbench should really be extracted into its own directory. This prevents anything within the vdbench tar from overwriting parts of the charm.

Comments

*You probably want vdbench output to go into a new directory per run to allow the case that somebody wants to rerun vdbench (presumably by redeploying to the same machine). Creating a directory for the current date/time would suffice.

*Be great if you could specify a custom vdbench parameter file to run. Either via config.yaml option(s) or by including something in the charm.

*I'd remove '-x' option from your shell scripts. You don't need to fill the Juju logs with debug output.

*Your copyright refers to non-existant files. You may also want to update the copyright year :)

Please fix the bugs and consider the comments and resubmit.

Thanks for your work!