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

Refactor valve types #415

Merged

Conversation

h-haaks
Copy link

@h-haaks h-haaks commented Nov 9, 2020

No description provided.

@h-haaks h-haaks requested a review from a team as a code owner November 9, 2020 21:48
@puppet-community-rangefinder
Copy link

tomcat::config::context::valve is a type

Breaking changes to this file MAY impact these 1 modules (near match):

tomcat::config::server::valve is a type

Breaking changes to this file WILL impact these 2 modules (exact match):
Breaking changes to this file MAY impact these 1 modules (near match):

This module is declared in 8 of 575 indexed public Puppetfiles.


These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

@h-haaks h-haaks force-pushed the refactor-valve-types branch 4 times, most recently from ffb3efe to a5f36b3 Compare November 10, 2020 01:18
Make valve types accept the same valve related parameters.
Add array parameter that can be used to uniquely identify valves with the sameclassName.
@h-haaks
Copy link
Author

h-haaks commented Nov 10, 2020

@david22swan
My second attempt to resolve MODULES-10855.
I hope you have some time to look at it.

Copy link
Member

@david22swan david22swan left a comment

Choose a reason for hiding this comment

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

@h-haaks
Sorry for the wait on the review, missed the notification from Github.
Anyway overall this look's good to me and it all runs successfully against the release pipeline so I'm gonna approve it.
However due to its backwards incompatible nature I want an approve from another member of the team in order to merge it.

Copy link
Contributor

@carabasdaniel carabasdaniel left a comment

Choose a reason for hiding this comment

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

👍 Thank you for your contribution @h-haaks

@david22swan david22swan merged commit 0f9fd00 into puppetlabs:main Nov 23, 2020
@h-haaks
Copy link
Author

h-haaks commented Nov 23, 2020

Thanks @david22swan and @carabasdaniel
Could any of you please tell me if I'm supposed to close https://tickets.puppetlabs.com/browse/MODULES-10855 my self, or if it should be looked at by someone before closing.

@h-haaks h-haaks deleted the refactor-valve-types branch November 23, 2020 23:46
@h-haaks
Copy link
Author

h-haaks commented Nov 23, 2020

By the way @david22swan, why did you label this backwards-incompatible?
I don't think it is, because as my test show it's still ok to use the resource_name and resource_type params of tomcat::config::context::valve.
You'll just get a warning that tells you that it is deprecated.

@david22swan
Copy link
Member

@h-haaks
Yeah had labelled this wrong sorry.
Have changed it to feature and will close the ticket in a minute.
Thank's again for putting in the work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants