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-9912) ERB Template Errors #320

Conversation

RandomNoun7
Copy link
Contributor

Prior to this change, some ERB templates would cause jruby errors
on the master during catalog compilation. The templates contained code
that would attempt to modify the value of a frozen string.

This change modifies the template code to ensure that no frozen string
values are attempted to be modified.

It also makes slight fixes to the syntax in the templates to ensure that
each instance of this pattern is written more consistantly.

Prior to this change, some ERB templates would cause jruby errors
on the master during catalog compilation. The templates contained code
that would attempt to modify the value of a frozen string.

This change modifies the template code to ensure that no frozen string
values are attempted to be modified.

It also makes slight fixes to the syntax in the templates to ensure that
each instance of this pattern is written more consistantly.
@glennsarti
Copy link
Contributor

Looks good but is missing a test for the behaviour?

Although does https://github.com/puppetlabs/puppetlabs-sqlserver/blob/master/spec/defines/user/permissions_spec.rb#L97-L111 cover it?

@RandomNoun7
Copy link
Contributor Author

image

@glennsarti
Copy link
Contributor

As per conversation with @RandomNoun7 , there is some unit testing for upcasing, however what is really needed is a full audit. This work is being tracked in https://tickets.puppetlabs.com/browse/MODULES-9917.

Happy for merge now.

@glennsarti glennsarti merged commit bad8ab5 into puppetlabs:master Sep 17, 2019
@RandomNoun7 RandomNoun7 deleted the tickets/MODULES-9912-fix-template-frozen-string-error branch September 17, 2019 03:34
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