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

loader: require .json extension and allow .yaml extension #520

Merged
merged 2 commits into from
Nov 2, 2023

Conversation

rmilecki
Copy link
Collaborator

@rmilecki rmilecki commented Aug 6, 2023

loader: allow YAML templates with .yaml extension

Oficially suggested extension for YAML files is .yaml. Accept it.
loader: require .json extension for JSON files

Trying to parse every file other than *.yml as JSON file isn't the best
idea. It wastes time on trying to parse non-template files. It produces
warnings like "json Loader Failed to load (...)template" for files that
were never meant to be templates. It's a bad pracice in general.

Check for file extension before using json.loads().

@bosd
Copy link
Collaborator

bosd commented Aug 7, 2023

Thanks for this PR.
I agree the loader could use some work. It's on my wish list :)
As the recent refactoring broke the possibility to stream load templates.

I think adding more extension filters will block the possibility to streamload templates even further.

Rafał Miłecki added 2 commits November 2, 2023 19:10
Trying to parse every file other than *.yml as JSON file isn't the best
idea. It wastes time on trying to parse non-template files. It produces
warnings like "json Loader Failed to load (...)template" for files that
were never meant to be templates. It's a bad pracice in general.

Check for file extension before using json.loads().

Fixes: 0d2d16c ("Native JSON Template Support")
Signed-off-by: Rafał Miłecki <[email protected]>
Oficially suggested extension for YAML files is .yaml. Accept it.

Link: https://yaml.org/faq.html
Fixes: invoice-x#514
Signed-off-by: Rafał Miłecki <[email protected]>
@bosd bosd merged commit f4e984f into invoice-x:master Nov 2, 2023
5 checks passed
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