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

Refs #30436 - add import/export params #297

Merged
merged 1 commit into from
Dec 7, 2020

Conversation

jeremylenz
Copy link
Contributor

This is the "second PR" from theforeman/puppet-pulpcore#147 (review)

Copy link
Contributor

@wbclark wbclark left a comment

Choose a reason for hiding this comment

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

Thanks for kicking this off @jeremylenz !

What we also need here is to declare, set a default for, and document the new parameter we are introducing in this module. 1d2eef1 is a nice and recent example of exactly what that looks like.

manifests/init.pp Outdated Show resolved Hide resolved
manifests/init.pp Show resolved Hide resolved
manifests/init.pp Outdated Show resolved Hide resolved
manifests/params.pp Outdated Show resolved Hide resolved
@jeremylenz
Copy link
Contributor Author

@wbclark updated

manifests/params.pp Outdated Show resolved Hide resolved
@jeremylenz
Copy link
Contributor Author

I think the test failures are because theforeman/puppet-pulpcore#147, which defines the allowed_export_path param, isn't merged yet.

@jeremylenz jeremylenz force-pushed the 30436-allowed-exports branch from 2e8bb71 to af28412 Compare December 2, 2020 16:00
manifests/init.pp Outdated Show resolved Hide resolved
manifests/init.pp Outdated Show resolved Hide resolved
@jeremylenz
Copy link
Contributor Author

@ekohl any other concerns?

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

It needs to update the dependency on puppet-pulpcore in metadata.json. I intend to do a release of it, but first theforeman/puppet-pulpcore#150 should be merged. IMHO that should have been the implementation in the first place, but it was merged before I could correct it.

@ekohl
Copy link
Member

ekohl commented Dec 3, 2020

I opened theforeman/puppet-pulpcore#151. Please update metadata.json to at least require 2.2.0.

@jeremylenz
Copy link
Contributor Author

@ekohl Thanks, updated

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Please squash the commits.

manifests/init.pp Outdated Show resolved Hide resolved
@ehelms ehelms merged commit ca43d25 into theforeman:master Dec 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants