Skip to content

Deprecate ports parameter as a string - #611

Open
traylenator wants to merge 1 commit into
puppetlabs:mainfrom
traylenator:breakingports
Open

Deprecate ports parameter as a string#611
traylenator wants to merge 1 commit into
puppetlabs:mainfrom
traylenator:breakingports

Conversation

@traylenator

@traylenator traylenator commented May 22, 2024

Copy link
Copy Markdown

Summary

Previously it was possible to set the ports parameter as:

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

Since #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.

Checklist

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

@LukasAud

Copy link
Copy Markdown

Hi @traylenator, there seems to be some conflicts with the PR. If you could resolve those, I'll be happy to kick of testing and get this moving.

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.
Comment thread manifests/frontend.pp
},
String $instance = 'haproxy',
String[1] $section_name = $name,
Optional[Boolean] $sort_options_alphabetic = undef,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It looks like the PR contains a regression for a previous PR (#573). Could you adjust this?

Comment thread manifests/listen.pp
},
String $instance = 'haproxy',
String[1] $section_name = $name,
Optional[Boolean] $sort_options_alphabetic = undef,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same regression here.

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.

4 participants