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

Few changes #1

Closed
wants to merge 1 commit into from
Closed

Few changes #1

wants to merge 1 commit into from

Conversation

apenney
Copy link

@apenney apenney commented Mar 7, 2012

(The typo is the most useful bit of this, I included the mysql gem stuff despite the fact I haven't done the work to ensure it's only relevant for rhel, and uses a params.pp to decide if to include and all that just to get it checked in for now. If I was smarter I'd know how to break this pull request apart into just the changes I want but I made them as one big commit).

  • Include params in config/enc.pp as I include this on puppetmasters
    that do not run foreman.
  • Include the mysql gem by default (this will need to be improved
    by someone to have a boundary for RHEL as it won't work on Debian)
  • Fix a typo in the external_node.rb.erb template.
  • Change latest to present.

* Include params in config/enc.pp as I include this on puppetmasters
  that do not run foreman.
* Include the mysql gem by default (this will need to be improved
by someone to have a boundary for RHEL as it won't work on Debian)
* Fix a typo in the external_node.rb.erb template.
* Change latest to present.
require => Class['foreman::install::repos'],
notify => Class['foreman::service'],
}

package{[ "mysql-devel", "gcc", "ruby-devel" ]:
ensure => present,
}
Copy link
Member

Choose a reason for hiding this comment

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

i think this stuff should not be required any more (using the new rpms?)

@GregSutcliffe
Copy link
Member

@apenney - Ohad's right, those package resources shouldn't be required any more. Everything else is great, can you update the pull request?

@@ -4,7 +4,7 @@ SETTINGS = {
:url => "<%= scope.lookupvar('foreman::params::foreman_url')%>",
:puppetdir => "<%= scope.lookupvar('foreman::params::puppet_home')%>",
:facts => "<%= scope.lookupvar('foreman::params::facts')%>",
:storeconfigs => "<%= scope.lookupvar('foreman::params::storeconfig')%>",
Copy link
Member

Choose a reason for hiding this comment

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

actually, this looks wrong, it should be a boolean and not a string, e.g.
:facts => true, vs :facts => "true"

@ohadlevy
Copy link
Member

I've clean it up and merged it.
thanks

@ohadlevy ohadlevy closed this May 22, 2012
mcanevet referenced this pull request in mcanevet/puppet-foreman Mar 13, 2013
Resync with theforeman's github
jistr referenced this pull request in jistr/puppet-foreman May 27, 2013
Use SCL paths for config files
jturel pushed a commit to jturel/puppet-foreman that referenced this pull request May 1, 2018
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