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

Add array_element #23

Merged
merged 1 commit into from
Sep 11, 2015
Merged

Conversation

underscorgan
Copy link

This allows elements in arrays in hocon files to managed individually.

@@ -112,7 +112,22 @@ hocon_setting { 'sample map setting':
type => 'array',
}
```


If you are trying to manage single entries in an array (for example, adding to an array from a define) you will need tot set the `'type'` parameter to
Copy link
Contributor

Choose a reason for hiding this comment

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

s/tot/to

@underscorgan
Copy link
Author

@fpringvaldsen fixed

tmp_val = []
tmp_val << value
tmp_val << value_to_set
tmp_val.flatten!
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if we should be calling flatten! and uniq! on the array here? HOCON supports duplicate array elements as well as nested arrays, so I could see this possibly changing a user's configuration in unexpected ways.

Copy link
Author

Choose a reason for hiding this comment

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

My concern with not having uniq! for the array_element is idempotence. Since the array can't be rewritten every time we'd just end up with duplicate elements each run. I think if I iterate through value and value_to_set I can get rid of flatten! though.

@MSLilah
Copy link
Contributor

MSLilah commented Sep 10, 2015

@mhaskel Thanks! I'm 👍 on this aside from calling flatten! and uniq!.

@jpinsonault or @cprice404 could one of you take a look at this PR?

This allows elements in arrays in hocon files to managed individually.
@underscorgan
Copy link
Author

@fpringvaldsen I was able to get around flatten! and uniq! \o/

@MSLilah
Copy link
Contributor

MSLilah commented Sep 11, 2015

\o/ :+1:

@cprice404
Copy link

This seems good if we think this will be a common use case, but if it's being implemented specifically in support of the authorization rules stuff, I'm not quite understanding how it will deal with sort order? The sort order of the array will be important.

@underscorgan
Copy link
Author

@cprice404 I think this would be generally useful. WRT sorting I haven't thought of a more generic way to implement that yet, it seems like each use is going to have some specific needs.

@cprice404
Copy link

Cool, I'm +1 on it as a "generally useful" thing, just wanted to make sure the sorting requirement is still being addressed for the other module.

cprice404 added a commit that referenced this pull request Sep 11, 2015
@cprice404 cprice404 merged commit 5432617 into puppetlabs:master Sep 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants