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

Stop specifying $name as the default host value #631

Merged
merged 3 commits into from
Jan 19, 2017

Conversation

jamtur01
Copy link
Contributor

This should not be the default, it doesn't make sense for localhost
bound JMX connections.

This should not be the default, it doesn't make sense for localhost
bound JMX connections.
@@ -24,8 +24,8 @@
puppetversion: Puppet.version,
facterversion: Facter.version
}
default_facts.merge!(YAML.load(File.read(File.expand_path('../default_facts.yml', __FILE__)))) if File.exist?(File.expand_path('../default_facts.yml', __FILE__))
default_facts.merge!(YAML.load(File.read(File.expand_path('../default_module_facts.yml', __FILE__)))) if File.exist?(File.expand_path('../default_module_facts.yml', __FILE__))
default_facts.merge!(YAML.safe_load(File.read(File.expand_path('../default_facts.yml', __FILE__)))) if File.exist?(File.expand_path('../default_facts.yml', __FILE__))
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

@jamtur01 hrm, we just reverted the safe_load calls earlier today. I think this may need a modulesync in the morning, and this change dropped, before we merge this.

Copy link
Contributor Author

@jamtur01 jamtur01 Jan 19, 2017

Choose a reason for hiding this comment

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

I pulled master before I created this - that update wasn't in there - given the length of the time it takes the run the tests it's kinda annoying this isn't sync'ed. :(

@jamtur01
Copy link
Contributor Author

The test failure here is unrelated to the branch.

@rnelson0
Copy link
Sponsor Member

It is annoying, and I'm sorry for that! We are trying to do better with the modulesyncs. Thank you for your patience.

This does look good to me and we can merge it once the other tests all pass, then I can do the modulesync in the morning to fix the build status. Thanks!

@rnelson0 rnelson0 merged commit a83e1c9 into voxpupuli:master Jan 19, 2017
@rnelson0
Copy link
Sponsor Member

And it completed while I was typing. Boom!

@rnelson0
Copy link
Sponsor Member

@jamtur01 as an aside, if there is something we can do to speed up the tests, please open an issue and let us know!

@bastelfreak bastelfreak added the enhancement New feature or request label Jan 29, 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