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

(MODULES-10471) Allow namevar to have special chars #148

Merged
merged 2 commits into from
Feb 15, 2020
Merged

(MODULES-10471) Allow namevar to have special chars #148

merged 2 commits into from
Feb 15, 2020

Conversation

jarretlavallee
Copy link
Contributor

Prior to this PR, the namevar in the dsc type allowed a subset of
characters. This was not a functional limit and did not need to be in
place. This PR removes this restriction and ensure the name is a
string.

This PR also fixes the spec tests for the resource_name as
they were only testing the name parameter. This just updates it to test the resource_name and removes the spec tests for the special character limitations.

Prior to this commit, the namevar in the dsc type allowed a subset of
characters. This was not a functional limit and did not need to be in
place. This commit removes this restriction and ensure the name is a
string. This commit also fixes the spec tests for the `resource_name` as
they were only testing the `name` parameter.
@jarretlavallee jarretlavallee requested a review from a team as a code owner February 13, 2020 17:48
@jarretlavallee
Copy link
Contributor Author

Please note that I have not run this through any acceptance testing. Does this repo have litmus enabled?

@codecov-io
Copy link

codecov-io commented Feb 13, 2020

Codecov Report

Merging #148 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #148   +/-   ##
=======================================
  Coverage   56.72%   56.72%           
=======================================
  Files           7        7           
  Lines         275      275           
=======================================
  Hits          156      156           
  Misses        119      119
Impacted Files Coverage Δ
lib/puppet/type/dsc.rb 92.98% <100%> (+1.75%) ⬆️
lib/puppet/provider/base_dsc_lite/powershell.rb 33.33% <0%> (-1.2%) ⬇️
...b/puppet_x/puppetlabs/dsc_lite/dsc_type_helpers.rb 33.33% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6915b70...109ef59. Read the comment docs.

@sheenaajay
Copy link
Contributor

Thanks @jarretlavallee for submitting the PR. Yes, its litmus ported. But will run a release checks on all platforms too. Thank you.

@sheenaajay
Copy link
Contributor

Screen Shot 2020-02-14 at 09 40 34

sheenaajay
sheenaajay previously approved these changes Feb 14, 2020
Prior to this commit, the resource_name spec was not specific to the name of the resource. This commit updates the regex to check for the name of the resource.

Co-Authored-By: sheenaajay <sheena@puppet.com>
@jarretlavallee
Copy link
Contributor Author

Thanks @sheenaajay. I merged the suggested change.

@sheenaajay sheenaajay merged commit a4ae353 into puppetlabs:master Feb 15, 2020
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.

3 participants