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

support new module creation with removed files in configuration #710

Closed
wants to merge 1 commit into from
Closed

support new module creation with removed files in configuration #710

wants to merge 1 commit into from

Conversation

bond-os
Copy link

@bond-os bond-os commented Jul 19, 2019

When running pdk new module against template that has a file set with delete: true in config_defaults.yml module creation fails as destination status returned from template is ignored and pdk tries to read non-existent file.

@coveralls
Copy link

coveralls commented Jul 19, 2019

Coverage Status

Coverage increased (+0.002%) to 92.067% when pulling c034a72 on bond-os:new-module-with-file-deleted into c60eea9 on puppetlabs:master.

…e:true set in config_defaults.yaml without error
@scotje scotje added the needs-triage Newly created issue that has not been reviewed by a PDK contributor label Aug 1, 2019
@binford2k binford2k requested a review from scotje August 1, 2019 21:05
@seanmil
Copy link
Contributor

seanmil commented Aug 7, 2019

Independently I had put up PR #723 before stumbling across this PR which I initially deemed to be more correct and complete. But then I realized that this PR does string comparisons to the status value, not symbol comparisons. The values yielded by PDK::Module::TemplateDir.render() here https://github.com/puppetlabs/pdk/blob/master/lib/pdk/module/templatedir.rb#L125-L130 are symbols. I have put up PR #725 which hopefully includes the best of this PR and my other PR.

@scotje
Copy link
Contributor

scotje commented Aug 7, 2019

I agree that I think #725 is a more correct fix for this, but thank you for getting the ball rolling @bond-os !

@scotje scotje closed this Aug 7, 2019
@bond-os bond-os deleted the new-module-with-file-deleted branch August 8, 2019 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-triage Newly created issue that has not been reviewed by a PDK contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants