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

remove the include ::apt #369

Merged
merged 3 commits into from
Apr 3, 2017
Merged

Conversation

damoxc
Copy link
Contributor

@damoxc damoxc commented Mar 17, 2017

Having the include ::apt in this module then prevents someone from defining their own instance of the apt class with custom parameters in another manifest, forcing the use of hiera to configure the apt class (unless of course this is the intention...).

Having the `include ::apt` in this module then prevents someone from defining
their own instance of the apt class with custom parameters, forcing the use of
hiera to configure the apt class.
Copy link
Contributor

@vinzent vinzent left a comment

Choose a reason for hiding this comment

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

while I like the idea of removing repo handling from the module this will maybe produce other problems because it does not include apt but uses resources like apt::source

IMHO repo handling should be done at profile level (with roles/profiles design) or be completely optional.

@damoxc
Copy link
Contributor Author

damoxc commented Mar 17, 2017

I guess the better way to handle this would be to include a manage_apt option set to true (so as not to interfere with the current behaviour), which then provides someone with a choice.

@bastelfreak
Copy link
Member

Thanks for the PR @damoxc!

@bastelfreak bastelfreak merged commit e24f536 into voxpupuli:master Apr 3, 2017
@bastelfreak bastelfreak added the enhancement New feature or request label Apr 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants