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

Replace poyo with pyyaml. #1489

Merged
merged 5 commits into from Apr 27, 2021

Conversation

dHannasch
Copy link
Contributor

@dHannasch dHannasch commented Jan 12, 2021

I was about to open an issue on poyo when I realized there's a much more fundamental question I should be asking: given that pyyaml was "adopted" two years ago, does it still make sense to maintain an entire separate YAML parser for cookiecutter?

This came up a bit before. #1136 (comment)

I could be wrong, but I sort of had the impression nobody with cookiecutter ever really wanted to be in the business of YAML parsing in the first place, and that poyo was just an act of desperation --- IIRC, with the half-complete migration to BitBucket, it wasn't even clear where you would go to contribute any fixes you needed. At this point, pyyaml is owned by The YAML Project and lives on GitHub with cookiecutter: https://github.com/yaml/pyyaml

It seems like it might be worth folding poyo into pyyaml, if there's anything cookiecutter needs that pyyaml doesn't currently have.

At this point pip install pyyaml just works on Linux and Windows. I'm opening this as a PR rather than an issue to show what a small change we're really talking about here --- anecdotally, the drop-in replacement in this branch just works.

(This turned out a bit more complicated than just replacing the parsing call, but it's all to do with testing --- some tests were writing out test config files that, as it turned out, were not actually valid YAML. That's one of the things about having a separate hand-rolled parser; the language it actually represents tends to drift without anyone noticing, with the obvious implications for debugging.)

@insspb insspb added the enhancement This issue/PR relates to a feature request. label Apr 27, 2021
@ssbarnea ssbarnea requested a review from insspb Apr 27, 2021
@ssbarnea ssbarnea closed this Apr 27, 2021
@ssbarnea ssbarnea reopened this Apr 27, 2021
@ssbarnea ssbarnea merged commit 00f0f13 into cookiecutter:master Apr 27, 2021
16 of 17 checks passed
@insspb insspb added the breaking-change Marks an important and likely breaking change. Require update for major version label Apr 27, 2021
@simobasso simobasso mentioned this pull request May 15, 2021
cagonza6 pushed a commit to cagonza6/cookiecutter that referenced this pull request Jun 16, 2021
* Replace poyo with pyyaml.

* remove unused imports

* assert that the raised exception is directly caused by a YAML error.

* In YAML, use single quotes for unescaped backslashes.

* Use single quotes for unescaped backslashes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Marks an important and likely breaking change. Require update for major version enhancement This issue/PR relates to a feature request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants