Skip to content

(maint) Remove 'create' key for masterhttplog setting#3397

Closed
kylog wants to merge 1 commit intopuppetlabs:masterfrom
kylog:maint/cleanup-file-settings
Closed

(maint) Remove 'create' key for masterhttplog setting#3397
kylog wants to merge 1 commit intopuppetlabs:masterfrom
kylog:maint/cleanup-file-settings

Conversation

@kylog
Copy link

@kylog kylog commented Dec 6, 2014

This is the only setting that uses this key, and yet the
webrick setup_logger step already creates the file if needed.
So removing this will allow for some code cleanup in
FileSetting and derived classes.

This is the only setting that uses this key, and yet the
webrick setup_logger step already creates the file if needed.
So removing this will allow for some code cleanup in
FileSetting and derived classes.
@kylog
Copy link
Author

kylog commented Dec 6, 2014

I wanted to get some comments on this before refactoring FileSetting and friends. Thoughts anyone?

@joshcooper joshcooper added PL and removed PL labels Dec 6, 2014
@puppetcla
Copy link

CLA signed by all contributors.

@joshcooper
Copy link
Contributor

We were discussing file-based settings today in the PR triage relating to #3045. If you have a file-based setting, e.g. Puppet[:ldapcrtdir], which is external to puppet, e.g. it's not Puppet[:confdir], we don't want puppet to go off and try to create/manage the directory. This is possible today by marking the setting as string-based, modifying the places that use the setting to only use the file/directory if it exists. I think that is preferable than marking the setting as :type => file, :create => false, especially because Puppet::Setting::FileSetting#create_files? only applies to files, but not directories. So 👍

@joshcooper
Copy link
Contributor

@kylog what do you think about ticketing this and cleaning up FileSettings at the same time? If we merge just this, I'm not going to remember to do those other things.

@joshcooper
Copy link
Contributor

ping @kylog

@kylog
Copy link
Author

kylog commented Jan 8, 2015

Thanks for the reminder. I created https://tickets.puppetlabs.com/browse/PUP-3817. Not sure if I'm going to circle back to this one super soon (although it should be simple), so I'll just close this PR for now.

@kylog kylog closed this Jan 8, 2015
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