HEP-Puppet / puppet-apelpublisher

A Puppet module to set up an Apel Publisher
1 stars 5 forks source link

Apel joiner #2

Closed fschaer closed 10 years ago

fschaer commented 10 years ago

This branch :

kreczko commented 10 years ago

Hi Frederic,

Thanks a lot for the contribution. The tests are running fine (although they are very simple) and the changes all look reasonable. One thing for the future: If you have several config parts, could you please group them as manifests/config.pp (class <module name>::config) manifests/config/other.pp (class <module name>::config::other) as you've done it for SSM?

Do you want to do this change and add it to this pull request or shall I merge it and open an issue for this change for someone to pick up?

Cheers, Luke

fschaer commented 10 years ago

Hi Luke,

It took me a while to understand ;) If I’m correct, you’d like to see the config_mysql.pp go into config/mysql.pp, right ?

If so, I can do it in the current PR (branch), no problem – if I understood correctly, any commit I push to the feature branch will be added to the PR until you validate it.

Fred

kreczko commented 10 years ago

Hi Fred,

Yes, that is correct. Sorry for not being clear.

Any update you do to the current PR (the branch you submitted for it) will be propagated and listed here.

Luke

fschaer commented 10 years ago

Hi, done/pushed

kreczko commented 10 years ago

Hi Fred,

It seems you pushed it into your master branch instead of apel_joiner branch.

fschaer commented 10 years ago

Whoops ! I’ll merge back the change in 5 secs…

kreczko commented 10 years ago

OK, thanks!