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

fix a couple of problems with erl_ssl_path fact #609

Merged
merged 1 commit into from
Sep 6, 2017

Conversation

costela
Copy link
Contributor

@costela costela commented Sep 6, 2017

(this is a re-submission of #606)

Hi there,

This small PR fixes two small issues with the erl_ssl_path fact.
The first is pretty straightforward: avoid the .gsub call if we didn't actually get any data to mangle.
The second is a bit less clear-cut: drop the use of .with_env while calling erl. Newer versions of facter (>=3) AFAIK stopped supporting it (compare 2.x docs with 3.x docs) and it was unclear why we needed it in the first place. If I'm missing something and it is needed in some case, I'd be happy to find another workaround.

It also adds simple testing for erl_ssl_path.

Cheers

- with_env got dropped in facter >=3, so stop using it.
- only call gsub if actually run; This should avoid "undefined method
  `gsub!' for false:FalseClass"
- add simple test
@wyardley wyardley added the bug Something isn't working label Sep 6, 2017
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.

@costela Thanks for adding the test, seems like an issue with the command not getting stubbed is resolved now. This looks good to me.

@costela costela mentioned this pull request Sep 6, 2017
12 tasks
@wyardley wyardley changed the title fix a couple of problems with erl_ssl_path fact (2nd try) fix a couple of problems with erl_ssl_path fact Sep 6, 2017
@wyardley wyardley merged commit 01f2170 into voxpupuli:master Sep 6, 2017
@costela costela deleted the fix_erl_ssl_path branch September 6, 2017 14:49
@bostrowski13
Copy link
Contributor

@wyardley Yes, this is now fixed.

slm0n87 pushed a commit to slm0n87/puppet-rabbitmq that referenced this pull request Mar 7, 2019
fix a couple of problems with erl_ssl_path fact
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants