chore: replace deprecated serde_yaml with serde_yml#450
chore: replace deprecated serde_yaml with serde_yml#450anshul-garg27 wants to merge 3 commits intogoogleworkspace:mainfrom
Conversation
serde_yaml 0.9.34 is marked as deprecated by its author (the version string in Cargo.lock shows +deprecated). Replace with serde_yml which is the actively maintained community fork with the same API. Only 4 call sites in generate_skills.rs — drop-in replacement.
🦋 Changeset detectedLatest commit: 606f248 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 |
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 addresses the deprecation of the Highlights
Changelog
Activity
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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly replaces the deprecated serde_yaml crate with its maintained community fork, serde_yml. The changes are straightforward and correctly applied at all call sites. My main feedback is regarding the version specification for serde_yml in Cargo.toml. Using "0.0" is too broad and could lead to future build issues. I've suggested a more specific version to improve dependency stability.
Address Gemini review: "0.0" is too broad and acts like a wildcard. Pin to the exact minor version 0.0.12 for predictable builds.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request correctly replaces the deprecated serde_yaml crate with its maintained fork, serde_yml. This is a necessary maintenance update to ensure the project uses supported dependencies. The changes are applied consistently across the codebase. My review includes suggestions to improve error handling at the parsing call sites. By logging the specific parsing errors instead of a generic warning, debugging potential issues with the YAML files will be much easier, improving the tool's overall maintainability.
The else branches for personas.yaml and recipes.yaml parsing now include the actual error message (e.g. line/column info), making debugging much easier if the YAML ever has a syntax issue.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request replaces the serde_yaml crate with serde_yml across the project. This change includes updating the Cargo.toml dependency and modifying all serde_yaml::from_str calls in src/generate_skills.rs and its tests to use serde_yml::from_str. Additionally, error handling for YAML parsing failures in src/generate_skills.rs was enhanced to print the specific error message.
Summary
serde_yaml = "0.9.34"withserde_yml = "0.0"in Cargo.tomlsrc/generate_skills.rsfromserde_yaml::from_strtoserde_yml::from_strWhy
serde_yamlis deprecated by its author — the version string inCargo.lockshows0.9.34+deprecated. The crate will not receive security fixes or maintenance updates.serde_ymlis the actively maintained community fork with an identicalfrom_strAPI, making this a drop-in replacement.Test plan
serde_yamlreferences remain insrc/