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

render implicit yml multi doc file correctly #551

Merged
merged 4 commits into from
Sep 13, 2019
Merged

render implicit yml multi doc file correctly #551

merged 4 commits into from
Sep 13, 2019

Conversation

dturn
Copy link
Contributor

@dturn dturn commented Sep 10, 2019

What are you trying to accomplish with this PR?

Fix a bug where we fail to add --- to an implicit document if there are multiple documents in the file.

Fixes: #513

How is this accomplished?
Only test the implicitness of the first yaml doc in a file.

What could go wrong?
We start adding doc separators when we shouldn't and create empty docs.

@dturn dturn added the 🪲 bug Something isn't working label Sep 10, 2019
@dturn dturn requested review from a team and peiranliushop September 10, 2019 17:57
test/fixtures/partials/no-doc-seperator.yml.erb Outdated Show resolved Hide resolved
implicit = true
YAML.parse_stream(rendered_content, "<rendered> #{filename}") { |d| implicit = d.implicit }
implicit = []
YAML.parse_stream(rendered_content, "<rendered> #{filename}") { |d| implicit << d.implicit }
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@dturn dturn Sep 11, 2019

Choose a reason for hiding this comment

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

This bug happens because we take all the resources we're provided and turn it into a single output stream. So its important to have separators between each doc in the final output even if the input doc is implicit. The code you linked is for erb rendering a single file (which can contain partials) but it's important that we render it as requested by the user.

@dturn dturn requested a review from a team September 13, 2019 15:29
Copy link
Contributor

@lei-lo lei-lo 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 also update the name of the fixture to no-doc-separator?

@dturn dturn merged commit 0ee2531 into master Sep 13, 2019
@dturn dturn deleted the render-implicit branch September 13, 2019 17:03
@douglas douglas mentioned this pull request Sep 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪲 bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Render task parses an implicit yml file as explicit when there are multiple yaml resources
3 participants