Skip to content

Bug #12122 fix#37

Closed
carlasouza wants to merge 9 commits intopuppetlabs:masterfrom
carlasouza:master
Closed

Bug #12122 fix#37
carlasouza wants to merge 9 commits intopuppetlabs:masterfrom
carlasouza:master

Conversation

@carlasouza
Copy link
Copy Markdown

The lookup method cannot return nil in case there isn't an available answer. Instead, an empty data structure ("", [] or {}) will be returned. Also, calling hiera_array and hiera_hash should return all results from all backends.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this not a change in behaviour?

>> foo = {:foo => 'foo'}
>> bar = {:foo => 'bar'}
>> foo = bar.merge foo
 => {:foo=>"foo"}
>> foo = {:foo => 'foo'}
>> bar = {:foo => 'bar'}
>> foo.merge! bar
 => {:foo=>"bar"}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, you're right. The first value found shouldn't be overridden.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This fails if answer is nil:

err: undefined method `empty?' for nil:NilClass

empty? is also not defined for TrueClass or FalseClass it seems.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

You're right, thanks for pointing that out. I pushed a fix for it.

@kelseyhightower
Copy link
Copy Markdown

@carlasouza Looks like we should try and rebase this on master, then it will be ready for review. Let me know if you need help rebasing this.

@kelseyhightower
Copy link
Copy Markdown

@carlasouza This should now be fixed: e6d57c0411d8d3c97053752375c94dba40c2d2ab

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