Skip to content

to_yaml function added. #259

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

Closed
wants to merge 1 commit into from
Closed

Conversation

rdark
Copy link

@rdark rdark commented May 17, 2014

  • Makes use of ZAML.dump

* Makes use of ZAML.dump
@cyberious
Copy link
Contributor

If you could please add Docs in the readme, I like the idea, we just need an updated README

@jhoblitt
Copy link

jhoblitt commented Oct 9, 2014

I like the idea of this too.

Is there a policy about accepting new functions into stdlib without arity declared?

@hlindberg I believe puppet 4.x is planned to have only native json serialization, is that correct? So yaml support is stdlib would be orthogonal?

@hlindberg
Copy link
Contributor

I think zaml is being dropped for 4.0 (ping @zaphod42 )

@cyberious
Copy link
Contributor

Except where an individual has a reason to drop a Yaml in for File Content but build from a string in the manifest. Yaml lives beyond just Puppet and users might find this handy. @jhoblitt @hlindberg @zaphod42

@zaphod42
Copy link
Contributor

zaphod42 commented Oct 9, 2014

Yes, ZAML is an implementation of YAML that puppet uses internally. It isn't a good idea for other systems to depend on it. They can safely use YAML.dump instead.

We are going to be removing ZAML in puppet 4.

@zaphod42
Copy link
Contributor

zaphod42 commented Oct 9, 2014

Considering that the other functions in this family are parseyaml, parsejson, and loadyaml, wouldn't a more fitting name for this be dumpyaml?

@cyberious
Copy link
Contributor

That sounds like a valid name change for something like this.

@rdark
Copy link
Author

rdark commented Oct 10, 2014

Considering that the other functions in this family are parseyaml, parsejson, and loadyaml, wouldn't a more fitting name for this be dumpyaml?

Happy to update PR if that's the consensus?

@rdark
Copy link
Author

rdark commented Oct 10, 2014

Yes, ZAML is an implementation of YAML that puppet uses internally. It isn't a good idea for other systems to depend on it. They can safely use YAML.dump instead.

Can't quite remember the reason for using ZAML instead of YAML as it's been a while since I submitted this PR. I think the reason may have been ordering in the output making the tests easier to implement, but then again may have been for another reason entirely..

@cyberious
Copy link
Contributor

@rdark please update to use YAML instead of ZAML.

@underscorgan
Copy link
Contributor

Hi @rdark, thanks for the contribution! I'm going to close this PR for now since we haven't heard from you in a while. If this is still something you're interested in having added to the module please address the concerns in the comments and resubmit. Thanks again!

@mmckinst mmckinst mentioned this pull request Jul 28, 2016
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.

6 participants