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

port: Enforce Stdlib::Port datatype #1473

Merged
merged 1 commit into from
Aug 30, 2023
Merged

Conversation

bastelfreak
Copy link
Collaborator

Summary

Provide a detailed description of all the changes present in this pull request.

Additional Context

Add any additional context about the problem here.

  • Root cause and the steps to reproduce. (If applicable)
  • Thought process behind the implementation.

Related Issues (if any)

Mention any related issues or pull requests.

Checklist

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

@ekohl
Copy link
Collaborator

ekohl commented Aug 28, 2023

I don't mind this, but can we wait until there are more backwards incompatible changes lined up? Perhaps even implement a deprecation where it changes the tests to integers and generates a warning in the stable release that string ports will be removed in the next release?

@bastelfreak
Copy link
Collaborator Author

@ekohl we will soonish merge #1450 and that will be a major release. I wanted to change the port within that major release.

@bastelfreak
Copy link
Collaborator Author

I will see if we can do a minor release before merging #1450 , then we could release a warning

@ekohl
Copy link
Collaborator

ekohl commented Aug 28, 2023

@bastelfreak a minor release would be nice. There are some changes lined up and getting those out is good.

@bastelfreak
Copy link
Collaborator Author

let's merge and release #1474 before #1473

@bastelfreak
Copy link
Collaborator Author

This should be merged after the 9.2.0 release

@bastelfreak
Copy link
Collaborator Author

This is now ready for review and merge

Copy link
Collaborator

@SimonHoenscheid SimonHoenscheid left a comment

Choose a reason for hiding this comment

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

LGTM

smortex
smortex previously approved these changes Aug 30, 2023
@ekohl ekohl merged commit 8212c56 into puppetlabs:main Aug 30, 2023
37 checks passed
@bastelfreak bastelfreak deleted the port branch August 30, 2023 18:57
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.

5 participants