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

Remove archive require in rabbitmqadmin class #669

Merged
merged 3 commits into from
Dec 4, 2017

Conversation

lzecca78
Copy link
Contributor

@lzecca78 lzecca78 commented Dec 4, 2017

due to the dependency of the module from puppet-archive, there is no need to make the require. Is also really hard to debug, because the puppet exception doesn't clearly refers to the module requirement (duplicate declaration)

Due to the dependency of the module from `puppet-archive`, there is no
need to make the require. Is also really hard to debug, because the
puppet exception doesn't clearly refers to the module requirement
(duplicate declaration)
    due to the dependency of the module from `puppet-archive`, there is
no need to make the require. Is also really hard to debug, because the
puppet exception doesn't clearly refers to the module requirement
(duplicate declaration)
@wyardley
Copy link
Contributor

wyardley commented Dec 4, 2017

[edited your post to include the text from your commit message]
Can you show the error you're getting?

If not using puppet module install, the module won't be installed, so I personally think the require is sensible.

Also, in theory, archive could / should become a soft dep now that some distributions are packaging it (though since the common distros don't, probably won't do this now).

@lzecca78
Copy link
Contributor Author

lzecca78 commented Dec 4, 2017

@wyardley , i think that is pointless the line require archive because the module already declares its dependency from puppet-archive. For this reason, if the module is missing, will broke both the archive{} define and the empy require archive as well. On top of this, there is no need to require archive without any further reasons like to call a non-default class parameter (for example: class{'::archive': aws_cli_install => true}). I lost half of my day to understand who was the other resource responsable of the duplicate declaration class, and at the end, i found it. I can understand if it was required for the logic above, but it isn't, because the resource defined will work also without the require.

@wyardley wyardley added the bug Something isn't working label Dec 4, 2017
@wyardley wyardley merged commit 79095ae into voxpupuli:master Dec 4, 2017
@wyardley
Copy link
Contributor

wyardley commented Dec 4, 2017

After some more discussion, it seems clear that require doesn't add much here, so I'm going to go ahead and merge this.
Thanks for the contribution.

@lzecca78
Copy link
Contributor Author

lzecca78 commented Dec 4, 2017

thank you @wyardley ! i think that this will save a lot of troubles to current and future users of this module!

@lzecca78
Copy link
Contributor Author

lzecca78 commented Dec 4, 2017

Can i ask you the last thing? is it possible to make a new release with this minor bugfix? I am currently blocked by this, if you can, you make my day!

cegeka-jenkins pushed a commit to cegeka/puppet-rabbitmq that referenced this pull request Mar 26, 2021
*     Removed require archive
due to the dependency of the module from `puppet-archive`, there is
no need to make the require. Is also really hard to debug, because the
puppet exception doesn't clearly refers to the module requirement
(duplicate declaration)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs-squash
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants