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

update puppet::defaults::{facterbasepath,reports_dir} #147

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Falkor
Copy link

@Falkor Falkor commented May 5, 2017

Signed-off-by: Sebastien Varrette [email protected]

@toepi
Copy link
Contributor

toepi commented May 31, 2017

sry to ask, but why do you think the defaults are wrong? I'm currently migrated some dynamic stuff from hiera as fact and is seem to work with puppet4 and this defaults - think after this pull request we get errors that /etc/puppetlabs/facter does not exists (or it is only on my system the case?)

@Falkor
Copy link
Author

Falkor commented Jun 1, 2017

my understanding of the puppet 4 locations is that /etc/puppetlabs/* is meant to host the [custom] configuration files (like puppet/puppet.conf, r10k/r10k.yaml etc.) while /opt/puppetlabs hosts wide-system things. We can argue on whether custom facts enter the first or second category, for me it was the first but if that breaks too much workflow with this module, just drop this proposal ;)

@rendhalver
Copy link
Member

I understand that the defaults may be different.
I am not arguing about that.
What I am saying is this module uses semantic versioning so I need keep the variables as is and only change them on a major release.
People who use this module are relying on it to manage their infrastructure in a predictable way and I won't release code that will break that.

@toepi
Copy link
Contributor

toepi commented Jun 1, 2017

@Falkor: thanks for this explanation.
@rendhalver: why not as optional parameter? so falkor can use his path and the old user use ours and your rethink it for the next major? For me the migration from 3 to 4 was an nightmare - but not an issue of this module.

@rendhalver
Copy link
Member

I am not sure which is actually the correct location.
I am not actually changing the defaults for the two vars you want to edit I am just setting them so I can reference them elsewhere in the code.
From what I have seen these actually work as-is.
Can you show me some error messages that show the locations I am using are actually wrong?

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.

3 participants