juanje / cookbook-redmine

Chef's Cookbook for installing Redmine
25 stars 21 forks source link

Refactoring #7

Closed telemaco closed 11 years ago

telemaco commented 11 years ago

Cookbook code refactoring. It includes:

juanje commented 11 years ago

This PR looks really good to me :smile:

I'll try and I'll give you feedback to see if we could merge to master. It's a big improvement for the cookbook :smile:

One thing, I've seen still some list of packages at the receipes that should be as attributes at the attributes/default.rb file.

Thanks for your good work here!

juanje commented 11 years ago

Hi @telemaco, are you going to put back the packages into the attibutes?

I think that and fix the few conflicts with master (your branch need a rebase from the current master, I believe) and the only reason not to merge the PR. The rest looks fine to me.

I'd like to merge this changes as soon as posible so the rest of PR can be processed.

Thanks for all the hard work :smile:

juanje commented 11 years ago

I've merged locally the changes and added some changes.

I can't merged here this PR into the master because of the confilcts, but the commits are in the merged branch => refactoring.

More work need to be done, but now I have the packages back to attribs and the recipe source working and tested.

btomasini commented 11 years ago

I know this issue is closed. I was wondering when these changes would be merged to master.

juanje commented 11 years ago

Hi @btomasini,

Sorry, I've been a bit busy lately. I wanted to testing a bit more and maybe add some things, but it could be ready for the merge if nobody find any issues. Have you try the refactory branch? Did you find any issues? Thanks

btomasini commented 11 years ago

I have tried the branch and have not found any issues. I simply ran it using vagrant up. Are there any other ways I should be testing it?

I did have to add the following to the Vagrantfile to get it to work. I am not sure if that is just something specific to my setup:

      :mysql => {
        :server_root_password => "supersecret_password",
        :server_debian_password => "supersecret_password",
        :server_repl_password => "supersecret_password"
      }

I have also run it successfully after merging the refactoring branch back to master.

juanje commented 11 years ago

Thanks for the feedback.

I'm trying to have tested with Test Kitchen most of the use cases: with package, from the source, with Ubuntu, with CentOS, with mysql and Postgresql. I think the most of them are working, but I got some issues with the postgresql one. I trying to fix it. Once it is done, I'll merge the branch.

If you like, you could try with test-kitchen or with the Vagrantfile, changing the attribs:

:redmine => {
  :database => {
    :production => {
      :adapter => 'postgresql'
    }
  }
}

or

:redmine => {
  :install_method => 'package'
}
juanje commented 11 years ago

It seems like that issue was a problem with my local version of Berkshelf. I'll check the doc and something else and I'll merge into master.

Thanks to both @telemaco and @btomasini for the work and the testing :smile:

juanje commented 11 years ago

I've just got the refactory branch merged into the master :smile:

Thanks again, guys