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

Adding case insensitive enum values #230

Merged
merged 3 commits into from
Nov 8, 2022

Conversation

nickgw
Copy link

@nickgw nickgw commented Nov 2, 2022

This change more closely mimics the functionality of the original dsc module, allowing enum values to be passed either as specified in the schema or their lowercase variants.

Resolves Issue #216

@nickgw nickgw requested a review from a team as a code owner November 2, 2022 18:05
@CLAassistant
Copy link

CLAassistant commented Nov 2, 2022

CLA assistant check
All committers have signed the CLA.

@chelnak
Copy link

chelnak commented Nov 2, 2022

@nickgw Epic work thank you.

I will get this reviewed tomorrow.

@nickgw
Copy link
Author

nickgw commented Nov 2, 2022

@chelnak thanks a bunch. Let me know if I should look into updating this with upped version numbers, changelog, etc (or if puppetlabs has any best practices around that for the repo).

@nickgw
Copy link
Author

nickgw commented Nov 7, 2022

Hey @chelnak, you get a chance to review this?

Copy link

@chelnak chelnak left a comment

Choose a reason for hiding this comment

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

Given that the matches on Puppets Enum type are case sensitive, I think what you have proposed is fine.

The change will hopefully help out a few consumers of Puppet.Dsc.

I can cut a release for the PowerShell module as soon as tomorrow but time to re-puppetize all existing modules is undefined for now.

We can look in to individual cases ad-hoc though.

https://puppet.com/docs/puppet/6/lang_data_abstract.html#:~:text=The%20Enum%20data%20type,-The%20Enum%20data&text=It%20takes%20any%20number%20of%20strings%2C%20and%20results%20in%20a,and%20requires%20at%20least%20one.

@chelnak
Copy link

chelnak commented Nov 8, 2022

@nickgw I will merge as soon as CI finishes.

@nickgw
Copy link
Author

nickgw commented Nov 8, 2022

@chelank, awesome, thank you! This is the list of Dsc resources I'd love to have hit:
activedirectorydsc,
certificatedsc,
computermanagementdsc,
dfsdsc,
dnsserverdsc,
networkingdsc,
octopusdsc,
psdscresources,
securitypolicydsc,
sqlserverdsc,
storagedsc,

Thanks again!

@chelnak chelnak merged commit cc8ba7b into puppetlabs:main Nov 8, 2022
@nickgw nickgw deleted the nickgw_allowdowncaseenum_216 branch November 8, 2022 23:33
@chelnak chelnak self-assigned this Nov 11, 2022
@chelnak chelnak added enhancement New feature or request bug Something isn't working and removed enhancement New feature or request labels Nov 11, 2022
@chelnak
Copy link

chelnak commented Nov 11, 2022

Hey @nickgw, happy Friday!

We've just cut 1.0.4 that contains this change 🥳

https://github.com/puppetlabs/Puppet.Dsc/releases/tag/1.0.4

Thanks again for your contribution - we will look at re-publishing modules next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants