Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Install Flag for Pre-Installed Diamond #18

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sercanaktas
Copy link

We are creating our OS images with every software products that we need are pre-installed. Then we use vagrant, which uses puppet to configure those software products. If we use the diamond package as is, we get duplicate declaration errors. That's why I introduced an install flag to skip the installation and just use the configuration. I think this change might be useful to other people as well.

Cheers,

Sercan Aktas

@@ -80,6 +83,7 @@
$handlers_path = undef,
$purge_collectors = false,
$install_from_pip = false,
$install = true,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd maybe name this something a bit more explicit, like manage_installation

@garethr
Copy link
Owner

garethr commented Dec 4, 2014

Thanks for submitting this, and apologies for taking a while to get back to you.

I'd be more than happy to see this included. Would you be able to add a simple rspec test to verify the functionality? Some details in the CONTRIBUTING file https://github.com/garethr/garethr-diamond/blob/master/CONTRIBUTING.md but I'm also happy to help with any questions. The tests help with making sure functionality doesn't get broken in future releases, especially with various people using the module and sending helpful pull requests like this one.

@sercanaktas
Copy link
Author

I am interested in making these changes and running the tests, but due to work schedule I won't be able to do them for a couple of weeks.

@garethr
Copy link
Owner

garethr commented Dec 4, 2014

Thanks, and no problem on the timing. Let me know if I can help.

@sercanaktas
Copy link
Author

Sure, thanks for the offer.

@sercanaktas
Copy link
Author

The work became super hectic, and I don't think I can do the tests anytime soon. Just giving a heads up :)

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

Successfully merging this pull request may close these issues.

2 participants