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

Use data-in-modules instead of params.pp #797

Merged
merged 4 commits into from
May 11, 2019
Merged

Use data-in-modules instead of params.pp #797

merged 4 commits into from
May 11, 2019

Conversation

dhoppe
Copy link
Member

@dhoppe dhoppe commented May 3, 2019

No description provided.

@dhoppe dhoppe changed the title Data in modules Use data-in-modules instead of params.pp May 3, 2019
@dhoppe dhoppe added needs-work not ready to merge just yet tests-fail labels May 3, 2019
@dhoppe dhoppe mentioned this pull request May 3, 2019
4 tasks
@dhoppe
Copy link
Member Author

dhoppe commented May 4, 2019

Do we really need to keep the server.pp? It seems to be some kind of wrapper for backward compatibility and this will be a major release.

@wyardley
Copy link
Contributor

wyardley commented May 4, 2019

I think it was already deprecated. Should be fine to pull it out.

@dhoppe dhoppe requested a review from wyardley May 4, 2019 19:57
@dhoppe dhoppe removed needs-work not ready to merge just yet tests-fail labels May 4, 2019
Copy link
Contributor

@wyardley wyardley left a comment

Choose a reason for hiding this comment

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

Sorry for the delay in review.

Looks good from a quick scan. I assume you generated REFERENCE.md via strings?

@dhoppe
Copy link
Member Author

dhoppe commented May 9, 2019

@wyardley Yes, I wanted to make sure that the default values are fetched correctly.

@wyardley
Copy link
Contributor

wyardley commented May 9, 2019

@dhoppe Yeah, forgot there isn't really a task to do that automagically yet... :/

@bastelfreak bastelfreak merged commit cc78a33 into voxpupuli:master May 11, 2019
apevec pushed a commit to redhat-openstack/rdoinfo that referenced this pull request May 13, 2019
@kobybr
Copy link

kobybr commented Aug 1, 2020

This should have been tagged as a breaking change. Previous parameter references have been used in control repos.
For example $rabbitmq::params::ssl_management_port is now $rabbitmq:::ssl_management_port.

@wyardley
Copy link
Contributor

wyardley commented Aug 1, 2020

Not much that can be done about it at this point almost a year later, even if that were the case, but I think params was always supposed to be private; I don’t think this breaks the documented suggested usage of this module.

And the class was marked as private well before this version was released

# @api private

Vox overall does a pretty good job of tracking breaking changes, but this is a volunteer effort and an open spruce product, so these things will happen.

@kobybr
Copy link

kobybr commented Aug 2, 2020

Yes, I see that was added in 8.4.1 - however, we implemented this module prior to that. As a volunteer effort I pointed this out not to complain, but to help for future releases. Anyways, thanks for responding it is much appreciated.

cegeka-jenkins pushed a commit to cegeka/puppet-rabbitmq that referenced this pull request Mar 26, 2021
Use data-in-modules instead of params.pp
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.

4 participants