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

stdlib v9 support, use namespaced stdlib functions #1208

Closed
wants to merge 6 commits into from

Conversation

ggabijaa
Copy link

@ggabijaa ggabijaa commented Dec 4, 2023

Pull Request (PR) description

Added Stdlib v9 support
Added Puppet v8 support

This Pull Request (PR) fixes the following issues

Fixes #1201

Copy link
Member

@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.

CI failures seems unrelated. Maybe a modulesync update will help in this regard.

metadata.json Outdated
@@ -22,7 +22,7 @@
},
{
"name": "puppetlabs/stdlib",
"version_requirement": ">= 4.13.0 < 9.0.0"
"version_requirement": ">= 4.13.0 < 10.0.0"
Copy link
Member

Choose a reason for hiding this comment

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

The namespaced version of merge() was added in 04d1f0c5cccb93d0bc928c7623c4b0c0ebda5b6b which require version 9.0.0 of the stdlib module:

Suggested change
"version_requirement": ">= 4.13.0 < 10.0.0"
"version_requirement": ">= 9.0.0 < 10.0.0"

Copy link
Author

Choose a reason for hiding this comment

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

updated accordingly, thanks

@ggabijaa ggabijaa requested a review from smortex December 6, 2023 11:41
@ldaneliukas
Copy link

@smortex any updates on this PR?

@td-adc
Copy link

td-adc commented Feb 13, 2024

Any updates/progress on this?

@@ -164,7 +164,7 @@
# }

# Generate Elasticsearch config
$data = merge(
$data = stdlib::merge(
Copy link
Contributor

Choose a reason for hiding this comment

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

I would really like to release a new version that allow 9.x before we start requiring 9.x
This makes it a bit easier to upgrade without breaking dependencies.

metadata.json Outdated Show resolved Hide resolved
@h-haaks
Copy link
Contributor

h-haaks commented Feb 21, 2024

@ggabijaa could you please rebase against voxpupuli:master so that it is ready for merge once the current changes on master is released?

@h-haaks
Copy link
Contributor

h-haaks commented Feb 21, 2024

Rename PR and squash commits to something like 'Use namespaced stdlib functions'?

@@ -21,7 +21,7 @@

$init_defaults = {
'MAX_OPEN_FILES' => '65535',
}.merge($elasticsearch::init_defaults)
}.stdlib::merge($elasticsearch::init_defaults)
Copy link
Member

Choose a reason for hiding this comment

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

you can add the hashes with +. then you don't need merge and don't need stdlib.

Copy link
Contributor

Choose a reason for hiding this comment

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

#1217 and just close this PR?

@bastelfreak bastelfreak mentioned this pull request Feb 21, 2024
@ggabijaa ggabijaa changed the title stdlib v9 and puppet v8 support stdlib v9 support, use namespaced stdlib functions Feb 21, 2024
@h-haaks
Copy link
Contributor

h-haaks commented Feb 21, 2024

@ggabijaa please have a look at #1217
We could either merge that or you need to remove the stdlib dependency from metatdata.yaml and clean up your commits.

@h-haaks
Copy link
Contributor

h-haaks commented Feb 21, 2024

Thanks for your effort on this @ggabijaa.
As I'm eager to get a new release out I decided to merge my PR and close this one.

@h-haaks h-haaks closed this Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

warnings when used with stlib 9.
6 participants