Charm needed : Openbravo

Reviewing now.

-Juan

Hi Khairul:

Thank you very much for the charm . This is very cool indeed!

I encourage you to run charm proof on your charm before submitting for review as it normally catches things that we all tend to forget from time to time.

I ran charm proof on your charm and this is what I found:

W: metadata name (openbravo3) must match directory name (openbravo) exactly for local deployment.
- Not really critical but it has been a convention that we would like to keep

E: Charms need a maintainer (See RFC2822) - Name <email>
- We now require that charms have a maintainer field in the metadata.yaml file. Yours would look something similar to:
  maintainer: Khairul Kamarudzzaman <email address hidden>

E: no copyright file
- We also need the charm to have a copyright file ( charm get mongodb for an example of a copyright file )

E: template relations should be renamed to fit charm: relation-name
E: template interface names should be changed: interface-name
E: template relations should be renamed to fit charm: relation-name
E: template interface names should be changed: interface-name
E: template relations should be renamed to fit charm: relation-name
E: template interface names should be changed: interface-name
- All of these need to be cleaned up in the metadata.yaml file ( rename the interfaces to something relevant to your charm ) and in the hooks directory ( each interface in metadata.yaml file should have a corresponding set of one or more hooks )

I also noticed that you hard-coded port 80 on install file. Consider adding a config.yaml file where you can put this and any other values that could have a default but, can also be changed by the users ( charm get mongodb for an example of how to use config.yaml in your charms )

I would be happy to review your charm once the above issues have been addressed. Just change the bug status to "Fix Committed" when done.

Thanks,

Juan

its pretty standard to use port 80 for web access ports. not sure that
really needs configuration, and more to the point, the http relation
interface does require it.

the larger issue for this charm, is that its not teasing apart the
relations of the service, its specific functionality is at the moment
installing a package. ie. The existing relation hooks and interfaces in the
 charm, are just template ones that serve no functional purpose.
At a minimum i would like to see this provide the 'http' interface and
require a 'postgres' database via a relation.

-k

On Thu, May 31, 2012 at 3:54 PM, Juan L. Negron
<email address hidden>wrote:

> Hi Khairul:
>
> Thank you very much for the charm . This is very cool indeed!
>
> I encourage you to run charm proof on your charm before submitting for
> review as it normally catches things that we all tend to forget from
> time to time.
>
> I ran charm proof on your charm and this is what I found:
>
> W: metadata name (openbravo3) must match directory name (openbravo)
> exactly for local deployment.
> - Not really critical but it has been a convention that we would like to
> keep
>
> E: Charms need a maintainer (See RFC2822) - Name <email>
> - We now require that charms have a maintainer field in the metadata.yaml
> file. Yours would look something similar to:
> maintainer: Khairul Kamarudzzaman <email address hidden>
>
> E: no copyright file
> - We also need the charm to have a copyright file ( charm get mongodb for
> an example of a copyright file )
>
> E: template relations should be renamed to fit charm: relation-name
> E: template interface names should be changed: interface-name
> E: template relations should be renamed to fit charm: relation-name
> E: template interface names should be changed: interface-name
> E: template relations should be renamed to fit charm: relation-name
> E: template interface names should be changed: interface-name
> - All of these need to be cleaned up in the metadata.yaml file ( rename
> the interfaces to something relevant to your charm ) and in the hooks
> directory ( each interface in metadata.yaml file should have a
> corresponding set of one or more hooks )
>
> I also noticed that you hard-coded port 80 on install file. Consider
> adding a config.yaml file where you can put this and any other values
> that could have a default but, can also be changed by the users ( charm
> get mongodb for an example of how to use config.yaml in your charms )
>
> I would be happy to review your charm once the above issues have been
> addressed. Just change the bug status to "Fix Committed" when done.
>
> Thanks,
>
> Juan
>
>
> ** Changed in: charms
> Status: New => Incomplete
>
> --
> You received this bug notification because you are a member of charmers,
> which is subscribed to the bug report.
> https://bugs.launchpad.net/bugs/997339
>
> Title:
> Charm needed : Openbravo
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/charms/+bug/997339/+subscriptions
>

Excerpts from Kapil Thangavelu's message of 2012-05-31 22:15:08 UTC:
> its pretty standard to use port 80 for web access ports. not sure that
> really needs configuration, and more to the point, the http relation
> interface does require it.
>

Minor correction, the http relation requires a 'port' argument, it is
never assumed to be port 80.

On Thu, May 31, 2012 at 6:34 PM, Clint Byrum <email address hidden> wrote:
>
>
> Minor correction, the http relation requires a 'port' argument, it is
> never assumed to be port 80.

thanks for clarifying.

Thanks juan .. Waiting for the feedback n comment ;)

On Friday, June 1, 2012, Juan L. Negron <email address hidden> wrote:

> Reviewing now.
>
> -Juan
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/997339
>
> Title:
> Charm needed : Openbravo
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/charms/+bug/997339/+subscriptions
>

Opps sorry .. Im viewing the 1st mail ... Btw ... Thanks to all for the
comment .. I am really appreciate for all the comment and feedback .. Ill
do the fixing base on all the comment given.. And it may take some time and
maybe a chat session if i might not understood the comment ..
Btw.. Thanks again n ill try to do my best ...

On Friday, June 1, 2012, Khairul Aizat Kamarudzzaman wrote:

> Thanks juan .. Waiting for the feedback n comment ;)
>
> On Friday, June 1, 2012, Juan L. Negron <<email address hidden><javascript:_e({}, 'cvml', '<email address hidden>');>>
> wrote:
>
>> Reviewing now.
>>
>> -Juan
>>
>> --
>> You received this bug notification because you are subscribed to the bug
>> report.
>> https://bugs.launchpad.net/bugs/997339
>>
>> Title:
>> Charm needed : Openbravo
>>
>> To manage notifications about this bug go to:
>> https://bugs.launchpad.net/charms/+bug/997339/+subscriptions
>>
>

Hi Khairul,

Any updates on this or should we closed this as abandoned? Thanks!