Skip to content

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