Skip to content
This repository has been archived by the owner on Jun 2, 2019. It is now read-only.

modify existing settings functionality to allow for multiple repos #14

Merged
merged 4 commits into from
Sep 5, 2013
Merged

modify existing settings functionality to allow for multiple repos #14

merged 4 commits into from
Sep 5, 2013

Conversation

dcrissman
Copy link
Contributor

This change is not backwards compatible in the sense that I remove the default_repo_config attribute from the settings.pp file in favour of allowing any number of repos to be configured. If this is an issue, I would be happy to re-add this variable so that both can co-exist. But I thought this approach was cleaner.

Open to suggestions.

@dcrissman
Copy link
Contributor Author

This is in line with #12.

@brettporter
Copy link
Contributor

Good change, but could it be made to maintain compatibility? Retain default_repo_config and insert it into the list of repos. Would also be good to have some specs to verify - though I haven't tried due to time at the moment, my suspicion is this would break those that we have.

@dcrissman
Copy link
Contributor Author

Committed a change to add the default_repo_config back in, however it is not yet tested. In theory it should work ;)

I created #16 as I am uncertain how to get the rspec tests running.

@dcrissman
Copy link
Contributor Author

rake spec
librarian-puppet install --path=spec/fixtures/modules/
/home/dcrissman/.rvm/rubies/ruby-1.9.3-p327/bin/ruby -S rspec spec/classes/maven_spec.rb spec/defines/environment_spec.rb spec/defines/settings_spec.rb --color
Warning: Config file /tmp/d20130816-6015-npu8rf/hiera.yaml not found, using Hiera defaults
.....................

Finished in 1.01 seconds
21 examples, 0 failures

@dcrissman
Copy link
Contributor Author

@brettporter I am re-submitting this as a merge request.

@jbcpollak
Copy link
Contributor

I realize after submitting it this is duplicate to #19 , except I made the repos global instead of inside the puppet-maven profile. I can see the benefit of both actually, although supporting both might be confusing.

... Update... Now I realize why you put the repositories into the profile... I forgot Maven doesn't support global repositories, so I've closed my PR.

@dcrissman
Copy link
Contributor Author

@brettporter Is there anything else you need from me for this?

@brettporter
Copy link
Contributor

just more hours in the day...

brettporter added a commit that referenced this pull request Sep 5, 2013
modify existing settings functionality to allow for multiple repos
@brettporter brettporter merged commit 7affbec into maestrodev:master Sep 5, 2013
@brettporter
Copy link
Contributor

This looks great - thanks for the patch, and for your patience!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants