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

Change existing Kafo type definitions to Puppet 4 types #114

Merged
merged 1 commit into from
Jan 11, 2017

Conversation

stbenjam
Copy link
Member

@stbenjam stbenjam commented Dec 1, 2016

No description provided.

@stbenjam stbenjam changed the title Change existing Kafo type definitions to Puppet 4 types [WIP] Change existing Kafo type definitions to Puppet 4 types Dec 1, 2016
@@ -27,4 +27,9 @@ PuppetLintParamDocs.define_selective do |config|
config.pattern = ["manifests/capsule.pp", "manifests/init.pp"]
end

require 'kafo_module_lint/tasks'
KafoModuleLint::RakeTask.new do |config|
config.pattern = ["manifests/capsule.pp", "manifests/init.pp"]
Copy link
Member

Choose a reason for hiding this comment

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

Should manifests/puppet.pp be included here as well? That should also be added to .sync.yml

@ekohl
Copy link
Member

ekohl commented Jan 6, 2017

This fixes an issue with nightlies so I'd like to get this merged.

@stbenjam stbenjam changed the title [WIP] Change existing Kafo type definitions to Puppet 4 types Change existing Kafo type definitions to Puppet 4 types Jan 9, 2017
@stbenjam
Copy link
Member Author

stbenjam commented Jan 9, 2017

Ready for another look. Puppet is not needed at the top level anymore, we did it a different way.

@ekohl
Copy link
Member

ekohl commented Jan 9, 2017

Is the removal of capsule intentional? It feels like it doesn't belong in changing the kafo type definitions. Maybe at least make it a separate commit?

#
# $certs_tar:: path to tar file with certs to generate. REQUIRED
# $certs_tar:: Path to tar file with certs to generate
Copy link
Member

Choose a reason for hiding this comment

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

A bit odd that this is required but in the code there's if $certs_tar {.

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right, it is actually optional since you don't need the certs tar on a Katello. I updated the type

Copy link
Member Author

Choose a reason for hiding this comment

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

I moved it here: #123

@stbenjam
Copy link
Member Author

Ok ready for another look

@stbenjam stbenjam merged commit b219b79 into theforeman:master Jan 11, 2017
@stbenjam stbenjam deleted the modulesync branch January 11, 2017 14:24
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.

2 participants