Skip to content

Conversation

Joris29
Copy link
Contributor

@Joris29 Joris29 commented Mar 24, 2023

Creating a PR to optimize some var types

Fixes #518

@Joris29 Joris29 requested a review from a team as a code owner March 24, 2023 08:38
@CLAassistant
Copy link

CLAassistant commented Mar 24, 2023

CLA assistant check
All committers have signed the CLA.

@puppet-community-rangefinder
Copy link

tomcat::config::server is a type

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

tomcat::install is a type

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

This module is declared in 8 of 580 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.

@jordanbreen28
Copy link
Contributor

jordanbreen28 commented Mar 24, 2023

Thanks for this @Joris29!
So the PR test failure was expected with the changing of data types & should be a simple fix. Can you run bundle exec puppet strings generate --format markdown --out REFERENCE.md

The spec test failures will be a bit more tricky, and look like the test cases will need amended or problems with the data types fixed to pass.

Appreciate the time you have spent on this!

@Joris29
Copy link
Contributor Author

Joris29 commented Apr 2, 2023

@jordanbreen28 Just to be clear should i also investigate the unit tests or will this be something done by you guys.
I'm more than happy to do it but just have to know don't want this to become a stale PR

@jordanbreen28
Copy link
Contributor

jordanbreen28 commented Apr 12, 2023

@jordanbreen28 Just to be clear should i also investigate the unit tests or will this be something done by you guys. I'm more than happy to do it but just have to know don't want this to become a stale PR

Apologies @Joris29 I was out last week and missed this notification!
It would be best to try figure out the test failures yourself, as if left like this it for the team it would need to be properly prioritised and added to our backlog before being assigned out, which could be quite some time.

I am happy to collaborate with you and your efforts as you work on this (when you have the time of course)!

@Joris29
Copy link
Contributor Author

Joris29 commented Apr 12, 2023

Alright i will take a look when i have time

@Joris29
Copy link
Contributor Author

Joris29 commented May 8, 2023

Made some changes and i ran the tests locally and they seem fine

@Joris29
Copy link
Contributor Author

Joris29 commented May 16, 2023

@jordanbreen28 Could you have a look?
Unit tests seem ok.

@jordanbreen28
Copy link
Contributor

@Joris29 so sorry I missed your ping!
Lets run the ci and see what happens

@jordanbreen28
Copy link
Contributor

This is looking close to go..
Can you clean up your commits (remove the merge ones mainly) and rebase, then we should be good to merge!
Nice job on this one @Joris29 💯

@Joris29
Copy link
Contributor Author

Joris29 commented May 24, 2023

@jordanbreen28 I did rebase to one clean commit so i guess it's good to go :)

@jordanbreen28
Copy link
Contributor

Great job @Joris29 - thanks for your work on this one..
I'll work to get this released this week.

@jordanbreen28 jordanbreen28 merged commit c389a85 into puppetlabs:main May 24, 2023
@Joris29 Joris29 deleted the Optimizing_var_types branch June 19, 2023 15:21
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.

Optimizing puppet var types

4 participants