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

feat: frontmatter #127

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

feat: frontmatter #127

wants to merge 6 commits into from

Conversation

maelle
Copy link
Member

@maelle maelle commented Dec 16, 2024

Fix #126

@maelle maelle requested a review from zkamvar December 16, 2024 14:59
# https://github.com/rstudio/blogdown/blob/9c7f7db5f11a481e1606031e88142b4a96139cce/R/utils.R#L407
# anotate seq type values because both single value and list values are
# converted to vector by default
yaml_load = function(x) yaml::yaml.load(
Copy link
Member Author

Choose a reason for hiding this comment

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

Was this used anywhere? Not in the tests at least 😅

Copy link
Member

@zkamvar zkamvar left a comment

Choose a reason for hiding this comment

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

Thank you! Sorry I haven't had time to get to this... I borked my home computer last week and haven't been able to get to this -_-

This looks good and pretty much is exactly what I was expecting! The only thing that is blocking is the regex for the JSON header was swallowing chunks at the beginning of documents.

Note that this will require some coordination with the carpentries to release because this will cause a lot of warnings (try testing pegboard with this release and you get over 70 warnings)

R/utils.R Outdated Show resolved Hide resolved
Co-authored-by: Zhian N. Kamvar <[email protected]>
@maelle
Copy link
Member Author

maelle commented Jan 10, 2025

@zkamvar the warnings you see in pegboard are the deprecations, correct?

@maelle
Copy link
Member Author

maelle commented Jan 10, 2025

I have the impression we're getting the deprecation warning even when not accessing YAML actually. Is there another example of an R6 class deprecating a class that you know of and that I could look at?

@maelle
Copy link
Member Author

maelle commented Jan 10, 2025

Or actually no 🤔 Not sure what I was seeing earlier.

@maelle maelle requested a review from zkamvar January 10, 2025 09:34
@maelle
Copy link
Member Author

maelle commented Jan 10, 2025

Forgot to write, thank you for the review!

@zkamvar
Copy link
Member

zkamvar commented Jan 10, 2025

@zkamvar the warnings you see in pegboard are the deprecations, correct?

Yes, those are the deprecation warnings. This is because pegboard's Episode class inherits yarn and was accessing the $yaml field directly.

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.

expand tinkr to reading metadata in toml?
2 participants