New charm: IBM MobileFirstServer
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Juju Charms Collection |
Triaged
|
Undecided
|
Unassigned |
Bug Description
Hi,
Here is a new charm for IBM MobileFirst Server for review.
Here is a new charm for IBM MobileFirst Server for review.
To test this charm we need the principle charm websphere-liberty as ibm-mobilefirst
Deployable ibm-mobilefirst
Deployable Charm Repo : https:/
And, its source code can be found in the below repository.
Source Code Repo : https:/
Deployable websphere-liberty charm can be found at https:/
Deployable ibm-db2 charm can be found at https:/
IBM-MobileFirst
link: https:/
IBM-MobileFirst
https:/
Thanks.
description: | updated |
description: | updated |
Changed in charms: | |
status: | In Progress → Fix Committed |
Changed in charms: | |
status: | Fix Committed → In Progress |
Changed in charms: | |
status: | In Progress → Fix Committed |
description: | updated |
# IBM MobileFirstServer review
Thank you for the submission of the IBM MobileFirstServer. I am sorry it took so long for someone to look at this charm. As always I am using the Charm Store Policy and Best Practices documents for reference when reviewing charms: https:/ /jujucharms. com/docs/ stable/ authors- charm-policy https:/ /jujucharms. com/docs/ stable/ authors- charm-best- practice
Here is my review.
## charm-proof
$ charm proof
I: all charms should provide at least one thing
I: missing recommended hook start
I would have expected this charm to provide something because it is a “server” in the end the deployment succeeded but I was not able to view the web pages, so it is hard for me to tell. Please consider the peer relation, if this software can be set up in a cluster mode.
## README.md /github. com/adam- p/markdown- here/wiki/ Markdown- Cheatsheet If you want to see how the HTML looks please view how the readme renders on the personal namespace section of the charm store: https:/ /jujucharms. com/u/ibmcharme rs/mobilefirst- server/ trusty Once the code is in bzr you can see the new revision in the charm store after a 30 minute update delay. Please note that code blocks must be indented in multiples of 4 for code blocks. In some cases 7 spaces are used, in other cases 2 spaces are used. Alternately you can use three back-ticks for code blocks (``` code here ```). Markdown has special handling for anything between the less than (<) and greater than (>) signs so “juju deploy --to <machine number> db2” converts to the following HTML “juju deploy --to db2” which is an invalid command. Revisit everything with the less than (<) and greater than (>) symbols to ensure they convert to HTML correctly.
The readme contains many misspelled words and formatting errors that do not convert to HTML correctly. Please refer to the Markdown cheat for formatting tips: https:/
The readme needs more specific steps in the setup of the apache2 server, and how to host the file, with examples of paths and file names so the Juju use has a chance to get this correct. As someone reading this charm for the first time I would really want more information about the MobileFirst product, what it does and what I can do with it after deployment.
The deploy instructions are incomplete if I followed those instructions db2 and websphere-liberty would not install because the licenses were not accpted. To install websphere-liberty and db2 users have to accept licenses. The readme doesn't have to cover all the details, but the readme should mention that other steps are needed to deploy the other software and the users should refer to their readme files.
## config.yaml
The configuration file looks good, I would expect to see better descriptions including file name examples in the description. For example if the default for a value is empty string, and the config-changed hook looking for predefined package names, the configuration description should provide those names so the users know what is a valid value.
## install
It looks like you are missing the -y on the first two apt-get install lines. Those could cause a problem in the future if they require other packages t...