Greetings IBM charmer team,

I've taken some time to review the IBM LSF platform charm, and I'm delighted to see the overall progress here. Its a great start to providing the LSF platform to consumers of the Juju Charm ecosystem.  While reviewing I compiled the following notes:

The charm passes proof, which is a brilliant way to start any review, Thank you for your attention to detail!
The icon looks of high quality, the metadata and copyright are within charm store guidelines.

There is description text however that will not render in the Juju GUI or in the charm store display. As markdown and the config.yaml are rendered to HTML, anything  rendered between angle brackets: < > will be interpreted by the browser as an HTML tag, and thus not rendered. The helper text here is problematic and needs to be refactored to identify variable output, without the brackets.

There is also a concern with ease of setup for new users using the Apache server as a deliverability mechanism, but i also know aisreal has started an effort to resolve this, and I will defer to those efforts as a resolution. As this directly effects the automated CI process. However while this is still the recommended path forward, please be explicit in the instructions to setup the apache repository. It wasn't directly ovious to me to just place the binaries in /var/www/html of a fresh apache2 installation on ubuntu 14.04 and point to that units ip address to serve the files. 

Looking at the test suite, 00-setup appears to be sniffing for configuration of lsf-url, and this requires manual intervention, while the 10-deploy test is looking in local.yaml for the values.  I see there's some preliminary work here to do so, but it requires you to manually edit 00-setup to set this value. I'm not filled with confidence that any of this is obvious to the user, or the ci system on how to continue on this path to a resolution.  

There also appears to be an error in 10-deploy test

ERROR: platform-lsf::10-deploy.py
[/tmp/bundletester-0ruIYd/platform-lsf/tests/10-deploy.py exit 1]
  File "/tmp/bundletester-0ruIYd/platform-lsf/tests/10-deploy.py", line 39
    installationpkg = config.get('platform-lsf').get('cfg_lsfinstall_pkg_name')
                                                                              ^
TabError: inconsistent use of tabs and spaces in indentation

To run these tests, i executed the following command:  bundletester -F -l DEBUG -v

you can install bundletester from pypi - pip install bundlestester, as this is the tooling we use to verify all charms in CI and when running manually.


Otherwise the charm appears to be in order, all the config is mutable, there's only a few minor nitpick things to resolve, and I'm confident that once these items are resolved, the platform-lsf charm will be ready for the charm store. Thank you for your submission, dedication, and patience during the review process.

I'm going to change status of this Bug to "In Progress" and when you're ready switch the bug status to 'Fix Committed' and someone will be along shortly to review your work.

If you have any questions/comments/concerns about the review contact us in #juju on irc.freenode.net or email the mailing list <email address hidden>, or ask a question tagged with "juju" on http://askubuntu.com.