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

Allow ports parameters as Stdlib::Ports #610

Merged
merged 1 commit into from
May 22, 2024

Conversation

traylenator
Copy link

@traylenator traylenator commented May 22, 2024

Summary

Currently the ports parameters must be specified as one of:

  • '1234'
  • '1234,5678'
  • ['1234']
  • ['1234','5678']

Allow it to also be specified as the Stdlib::Port type. e.g.

  • 1234
  • [1234]
  • [1234,5678]

To encourage this direction the documentation has been updated to use Stdlib::Port types.

The old formats are still supported.

Motivation here is we were happily using v4 of this module with integer specifications for ports and this is better anyway.

Some future version could drop the old stringified ports.

Checklist

  • 🟢 Spec tests.
  • 🟢 Acceptance tests.
  • Manually verified. (For example puppet apply)

Currently the ports parameters must be specified as one of:

* `'1234'`
* `'1234,5678'`
* `['1234']`
* `['1234','5678']`

Allow it to also be specified as the `Stdlib::Port` type.
e.g.

* `1234`
* `[1234]`
* `[1234,5678]`

To encourage this direction the documentation has been updated to use
`Stdlib::Port` types.

The old formats are still supported.

Motivation here is we were happily using v4 of this module with integer
specifications for ports and this is better anyway.

Some future version could drop the old stringified ports.
@traylenator traylenator requested review from bastelfreak and a team as code owners May 22, 2024 13:51
Copy link
Collaborator

@bastelfreak bastelfreak left a comment

Choose a reason for hiding this comment

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

Do I see this correctly that the old syntax is still valid, so it's not a breaking change?

@traylenator
Copy link
Author

Do I see this correctly that the old syntax is still valid, so it's not a breaking change?

Yep that should be the case and was my intention. Its a good candidate for a breaking change down the line but no urgency from me on that.

@bastelfreak
Copy link
Collaborator

@traylenator could you do me a favour and also provide a PR as breaking change that drops the other types? <3 Just so that we don't forget it for the next major release?

@bastelfreak bastelfreak merged commit dcfd35d into puppetlabs:main May 22, 2024
40 of 41 checks passed
@traylenator traylenator deleted the ports branch May 22, 2024 14:11
traylenator added a commit to traylenator/puppetlabs-haproxy that referenced this pull request May 22, 2024
Previously it was possible to set the ports parameter
as:

* `'1234'`
* `'1234,5678'`
* `['1234']`
* `['1234','5678']`

Since puppetlabs#610 the ports parameter could also be specified with `Stdlib::Ports`:

* `1234`
* `[1234]`
* `[1234, 5678]`

With this patch all the previous string values are deprecated and
will fail. The `Stdlib::Port` values must be used.
traylenator added a commit to traylenator/puppetlabs-haproxy that referenced this pull request May 22, 2024
Previously it was possible to set the ports parameter
as:

* `'1234'`
* `'1234,5678'`
* `['1234']`
* `['1234','5678']`

Since puppetlabs#610 the ports parameter could also be specified with `Stdlib::Ports`:

* `1234`
* `[1234]`
* `[1234, 5678]`

With this patch all the previous string values are deprecated and
will fail. The `Stdlib::Port` values must be used.
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.

3 participants