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 downcase() only for fact sets which fact a valid operatingsystem #4

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

baurmatt
Copy link

facterdb 0.8.1 introduced a SLES 15 SP1 fact set which doesn't have a
operatingsystem fact. This is due to bugs in facter 2.5. As facter 2.5
doesn't get maintance anymore, we need to fail safely here.

Fixes #3.

@baurmatt
Copy link
Author

Not sure why the 1.9 test fails. Pretty sure it's not because of my change.

@cardil
Copy link
Collaborator

cardil commented Aug 12, 2019

I've checked out develop branch and runned bundle exec rake spec on Ruby 1.9 and all tests have passed.

It must be your change that breaks this test on Ruby 1.9.

@cardil cardil self-requested a review August 12, 2019 15:47
Copy link
Collaborator

@cardil cardil left a comment

Choose a reason for hiding this comment

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

This change, breaks existing test for Ruby 1.9. Code should be adjusted to make all test pass. Additionally, we should make a test case to cover change you make.

facterdb 0.8.1 introduced a SLES 15 SP1 fact set which doesn't have a
operatingsystem fact. This is due to bugs in facter 2.5. As facter 2.5
doesn't get maintance anymore, we need to fail safely here.

Fixes coi-gov-pl#3.
@baurmatt
Copy link
Author

Hey @cardil,

thanks for taking a look! :) I've altered the output as suggested and added a test.

Regarding the failing tests: I'm pretty sure the error isn't related because the latest commit in the develop branch throws the same error, but for Ruby 2.4. Also my just pushed commit makes Ruby 1.9 fail, but let Ruby 2.1 fail.

@baurmatt
Copy link
Author

Hey @cardil could we get this merged and released? :)

@baurmatt
Copy link
Author

baurmatt commented Apr 8, 2020

Hey @cardil, any chance to get this merged and released? :)

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.

Fails with facterdb 0.8.1
2 participants