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

Allow for yaml aliases to be loaded using YAML.safe_load #510

Merged
merged 1 commit into from
Jul 9, 2019

Conversation

jmreid
Copy link
Member

@jmreid jmreid commented Jul 9, 2019

What are you trying to accomplish with this PR?
As of #504 and 0.26.5 , YAML aliases no longer load and instead throw a Psych::BadAlias.

This PR reenabled YAML aliases based on the Psych docs:
https://ruby-doc.org/stdlib-2.1.0/libdoc/psych/rdoc/Psych.html#method-c-safe_load

How is this accomplished?
Changes YAML.safe_load

What could go wrong?
Matches the YAML alias behaviour pre-0.26.5, so I'm hoping this is pretty safe. It looks like the dropping of YAML alias support wasn't intentional.

@jmreid jmreid requested review from JennaBlack and dturn July 9, 2019 04:18
@DazWorrall
Copy link
Member

I think we want this so added a test to prevent a future regression, I'll leave the approval to people with more context.

@jmreid jmreid requested a review from KnVerey July 9, 2019 16:14
Copy link
Contributor

@dturn dturn left a comment

Choose a reason for hiding this comment

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

Can you update CHANGELOG.md too. I'm 👍 on releasing 0.26.6 after this gets merged.

lib/kubernetes-deploy/bindings_parser.rb Outdated Show resolved Hide resolved
@jmreid jmreid merged commit 356ad94 into master Jul 9, 2019
@jmreid jmreid deleted the load-yaml-aliases branch July 9, 2019 16:44
@jmreid jmreid temporarily deployed to rubygems July 9, 2019 17:26 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants