Skip to content

Conversation

@danielparks
Copy link
Owner

No description provided.

@danielparks danielparks merged commit f6fc998 into main Oct 10, 2022
@danielparks danielparks deleted the absolute-requires branch October 10, 2022 22:25
@bastelfreak
Copy link

@danielparks Hi!
thanks for the awesomme module. Stupid question, could you revert this commit? With require_relative, environment isolation works. Puppet will ensure that the loaded lib comes from the same environment as the type and provider. With just using require, you potentially load the puppet_x lib from another code environment and cause some trouble. An example PR is puppetlabs/puppetlabs-stdlib#1275. We (voxpupuli.org) and Puppet are currently checking for a rubocop rule to even enforce require_relative.

@danielparks
Copy link
Owner Author

Sure, thanks for the explanation. Just out of curiosity, how did you find this?

@bastelfreak
Copy link

I'm one of the Vox Pupuli maintainers and we maintain a lot of modules (https://github.com/voxpupuli?q=puppet-&type=all&language=&sort=) :D. We noticed this in a few modules. This is usually a problem when a module is updated in a new environment and production still uses an older version, where some methods from a class in puppet_x aren't yet implemented.

@danielparks
Copy link
Owner Author

danielparks commented Oct 24, 2022

Ah, sure. :) I meant, how did you find this PR I made on this repo — it seems pretty obscure. :)

I opened a PR #36 to switch to require_relative, but I note that the stdlib PR you mentioned actually changes a require_relative in a way that looks like it shouldn’t make a difference.

-require_relative '../stdlib'
+require_relative '../../puppet_x/stdlib'

Should I adjust my require_relatives to back further up the directory tree and include the full puppet_x path? I have a hard time imagining why that would be necessary, but it sounds like it is.

Thanks!

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