Skip to content

(CONT-1023) - Enhancing deferrable_epp to support nested hash #1359

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

Merged
merged 2 commits into from
May 31, 2023
Merged

Conversation

Ramesh7
Copy link
Contributor

@Ramesh7 Ramesh7 commented May 29, 2023

Summary of Issue

Currently stdlib::deferrable_epp doesn't support well if the input variable is nested hash has Deferrable value .

Change

Adding support to evaluate the nested hash values to add support for nested hash.

@Ramesh7 Ramesh7 force-pushed the CONT-1023 branch 2 times, most recently from dea98f1 to 06bcac6 Compare May 29, 2023 17:37
@Ramesh7 Ramesh7 added WIP Work In Progress maintenance labels May 29, 2023
@Ramesh7 Ramesh7 removed the WIP Work In Progress label May 30, 2023
binford2k
binford2k previously approved these changes May 30, 2023
Copy link
Collaborator

@smortex smortex left a comment

Choose a reason for hiding this comment

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

This feels more more future proof to me, thanks! Only a minor fix and it's 💯 to me!

Copy link
Collaborator

@smortex smortex left a comment

Choose a reason for hiding this comment

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

LGTM!

@Ramesh7 Ramesh7 merged commit 4120eb2 into main May 31, 2023
@Ramesh7 Ramesh7 deleted the CONT-1023 branch May 31, 2023 03:48
#
# Output : ["value1", "value2.1"]
#
Puppet::Functions.create_function(:nested_values) do
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happened to all new functions being namespaced??

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggest namespacing it without creating a shim as this hasn't made it to a release yet.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Damned! Yes, to all of this 😪

describe 'nested_values' do
# please note that these tests are examples only
# you will need to replace the params and return value
# with your expectations
Copy link
Collaborator

Choose a reason for hiding this comment

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

These comments should not have made it in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed in #1363.

# frozen_string_literal: true

# This function will return list of Hash values, the return value will be Array
# NOTE : This function is expecting only Hash and return value will be Array
Copy link
Collaborator

Choose a reason for hiding this comment

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

These two lines could be reworded to remove the repetition about the return value.

Copy link
Contributor Author

@Ramesh7 Ramesh7 May 31, 2023

Choose a reason for hiding this comment

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

addressed here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants