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

parse also *.yml file suffix as YAML; closes #1958 #1959

Merged
merged 1 commit into from
Aug 10, 2022
Merged

parse also *.yml file suffix as YAML; closes #1958 #1959

merged 1 commit into from
Aug 10, 2022

Conversation

bast
Copy link
Contributor

@bast bast commented Aug 9, 2022

previously only files with *.yaml suffix were recognized as YAML

Questions to the reviewer:

  • Should I add more to the documentation?
  • Should I add anything to the changelog or change commit message format?
  • I have only tested this locally with load_data from a path ending with *.yml (and this worked). Should I add/extend some test?
  • I tried to also test this for URLs but I am unsure how the parsing works there. It does not seem to look at the URL suffix (tested with .toml and .yaml) but perhaps at the header. So I am unsure whether more is needed for URLs or whether they already worked and I did not have a good test. I tried with pages served via https://raw.githubusercontent.com/ but maybe they had wrong header.

Sanity checks:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Are you doing the PR on the next branch?
  • Have you created/updated the relevant documentation page(s)?

previously only files with *.yaml suffix were recognized as YAML
@Keats
Copy link
Collaborator

Keats commented Aug 9, 2022

I believe load_data with urls will basically require you to be explicit about the format you're loading rather than relying on headers. I'll double-check tomorrow.

No need to add tests, it's just a simple match.

@Keats
Copy link
Collaborator

Keats commented Aug 10, 2022

Yep, it looks like it just defaults to Plain for urls

@Keats Keats merged commit 86e5b6d into getzola:next Aug 10, 2022
@bast
Copy link
Contributor Author

bast commented Aug 11, 2022

Thanks for double-checking it. And the setting makes sense.

@bast bast deleted the load-data-yml-suffix branch August 11, 2022 06:31
Keats pushed a commit that referenced this pull request Aug 14, 2022
previously only files with *.yaml suffix were recognized as YAML
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