refactor: drop serde_yaml by migrating to toml#657
Conversation
🦋 Changeset detectedLatest commit: 9521fca The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #657 +/- ##
==========================================
+ Coverage 70.84% 70.86% +0.02%
==========================================
Files 44 44
Lines 20646 20644 -2
==========================================
+ Hits 14627 14630 +3
+ Misses 6019 6014 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #657 +/- ##
==========================================
+ Coverage 70.84% 70.86% +0.02%
==========================================
Files 44 44
Lines 20646 20644 -2
==========================================
+ Hits 14627 14630 +3
+ Misses 6019 6014 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request migrates the project's internal AI skill registry from YAML to TOML. The primary motivation for this change is to eliminate the unmaintained Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request migrates the internal AI skills registry, including personas and recipes, from YAML to TOML format. This change enables the removal of the unmaintained serde_yaml dependency, enhancing the project's security posture. Review feedback identifies a need to correct newline escaping in recipes.toml to ensure proper markdown formatting in generated outputs and suggests improving error reporting in generate_skills.rs by capturing and displaying TOML parsing errors instead of swallowing them.
| steps = [ | ||
| "Copy the template: `gws drive files copy --params '{\"fileId\": \"TEMPLATE_DOC_ID\"}' --json '{\"name\": \"Project Brief - Q2 Launch\"}'`", | ||
| "Get the new doc ID from the response", | ||
| "Add content: `gws docs +write --document-id NEW_DOC_ID --text '## Project: Q2 Launch\n\n### Objective\nLaunch the new feature by end of Q2.'`", |
There was a problem hiding this comment.
Inconsistent newline escaping in recipe steps. In TOML double-quoted strings, \n (single backslash) is interpreted as a literal newline character. This causes the generated SKILL.md to have broken markdown list formatting because the newline is not indented. To represent a literal \n sequence for the CLI command, use \\n (double backslash), as correctly done on line 226. This issue also affects lines 446 and 494.
"Add content: gws docs +write --document-id NEW_DOC_ID --text '## Project: Q2 Launch\\n\\n### Objective\\nLaunch the new feature by end of Q2.'",
| } | ||
| } else { | ||
| eprintln!("WARNING: Failed to parse personas.yaml"); | ||
| eprintln!("WARNING: Failed to parse personas.toml"); |
There was a problem hiding this comment.
The error message from the TOML parser is swallowed here, which makes it difficult to diagnose syntax errors in the personas.toml file. It is recommended to capture and print the error (e.g., by using a match statement or checking the Err variant). This also applies to the recipes parsing logic at line 262.
Translates the internal AI skill registry files (personas and recipes) from YAML to TOML. This completely eliminates the unmaintained serde_yaml dependency (which was failing supply chain audits) since toml is actively maintained and handles the static config generation perfectly.