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

Update Symfony/Yaml to v4, add support for YAML processing #16

Merged
merged 8 commits into from
Mar 25, 2021

Conversation

bennothommo
Copy link
Member

Fixes wintercms/winter#13

This PR upgrades symfony/yaml to version 4.4 to match the rest of the Symfony components in Storm. To support old version.yaml file types, this also adds processor support for the YAML parser, and adds a pre-processor that will fix up most issues in old version.yaml files so that they correctly parse.

I've tried to cover most bases where the new Symfony YAML parser might trip up, but if anyone can think of other cases, please let me know so I can add them to the test case.

@bennothommo bennothommo added this to the v1.1.3 milestone Mar 24, 2021
@bennothommo bennothommo changed the title Add support for version YAML pre-processing Add support for version.yaml pre-processing Mar 24, 2021
@LukeTowers
Copy link
Member

LGTM!

@LukeTowers
Copy link
Member

LukeTowers commented Mar 24, 2021

@bennothommo On second glance, shouldn't the VersionYamlProcessor be located in the system module? Makes it harder to test the processor logic here, but perhaps we can add a basic processor as a test fixture here and then the full version yaml processor test in the winter repo instead. Also the VersionManager needs to set the processor in order for this to take effect anyways.

@bennothommo
Copy link
Member Author

@LukeTowers sounds good - will sort that out, although I won't put a test in for Storm as we can assume Yaml is adequately tested by Symfony, and I can't think of another processor we need right now.

@bennothommo bennothommo changed the title Add support for version.yaml pre-processing Update Symfony/Yaml to v4, add support for YAML processing Mar 25, 2021
@bennothommo bennothommo merged commit a3273f3 into develop Mar 25, 2021
@bennothommo bennothommo deleted the wip/version-yaml-preprocess branch March 25, 2021 03:28
LukeTowers pushed a commit to wintercms/winter that referenced this pull request Mar 28, 2021
Fixes #13
Follows on from wintercms/storm#16

Moves the version.yaml processing into the System module and updates all version file parsing to use this new processor.
LukeTowers pushed a commit to wintercms/wn-system-module that referenced this pull request Mar 28, 2021
Fixes #13
Follows on from wintercms/storm#16

Moves the version.yaml processing into the System module and updates all version file parsing to use this new processor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants