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

Use Symfony YAML handler #52

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

distantnative
Copy link
Member

@distantnative distantnative commented Sep 1, 2024

🚨 Only merge after 4.5.0 release

Description

Summary of changes

  • Switches to Symphony YAML handler

Reasoning

Before we switch the default in the Core, it would be good to run our kits first with this handler. Gives us more chances to catch any issues/differences that the new handler has, which would then allow us to make the default switch easier for developers.

Copy link
Member

@bastianallgeier bastianallgeier left a comment

Choose a reason for hiding this comment

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

I've clicked through all the panel views and couldn't find any issues. I think it's a good idea to already activate the new handler here.

I wonder if we should add a comment to the config to explain why we use that setting and what it means? It could be helpful for starterkit users, to understand this some more.

@distantnative
Copy link
Member Author

@bastianallgeier thinking about it. Maybe we wait for 4.5 with getkirby/kirby#6638 included?

Yes to the comment.

@distantnative
Copy link
Member Author

What could be a good comment?

already makes use of the more modern Symfony YAML parser: https://getkirby.com/docs/reference/system/options/yaml (will become the default in a future Kirby version)

@bastianallgeier bastianallgeier added this to the 4.5.0 milestone Sep 10, 2024
@bastianallgeier
Copy link
Member

That comment suggestion is great!

@distantnative
Copy link
Member Author

Added the comment

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.

2 participants