Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR bumps yaml-cpp to 0.9.0, updates the Changesyaml-cpp 0.9.0 update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@yaml-cpp.sh`:
- Around line 9-10: The prefer_system_check here is missing the required shebang
which triggers ali:bad-shebang; update the heredoc for prefer_system_check so
its first line is the aliBuild-standard shebang (match the script's existing
shebang, e.g. #!/bin/sh) so the embedded script starts with that header; modify
the prefer_system_check block (symbol: prefer_system_check) to include the
shebang as its first line.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 31932245-501e-4857-a4f4-e85182ae6a0d
📒 Files selected for processing (2)
CHANGELOG.mdyaml-cpp.sh
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
antonioiuliano2
left a comment
There was a problem hiding this comment.
All clear to me, thanks!
Antonio
There was a problem hiding this comment.
♻️ Duplicate comments (1)
yaml-cpp.sh (1)
10-10:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winShebang still doesn't match aliBuild convention.
The previous review asked for the aliBuild-standard
#!/bin/bash -eheader, but the embedded script now uses#!/bin/sh.alidistlintcontinues to emitali:bad-shebangfor line 10, and the same applies to line 17.Suggested patch
prefer_system_check: | - #!/bin/sh + #!/bin/bash -e REQUESTED_VERSION=${REQUESTED_VERSION#yaml-cpp-} REQUESTED_VERSION=${REQUESTED_VERSION#v} @@ --- -#!/bin/sh +#!/bin/bash -eAlso applies to: 17-17
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@yaml-cpp.sh` at line 10, Replace the non-conforming shebangs in the shell scripts from "#!/bin/sh" to the aliBuild standard "#!/bin/bash -e": update the top-level shebang in yaml-cpp.sh and the embedded script shebang (the second occurrence currently at line 17) so both use "#!/bin/bash -e" to satisfy alidistlint's ali:bad-shebang check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@yaml-cpp.sh`:
- Line 10: Replace the non-conforming shebangs in the shell scripts from
"#!/bin/sh" to the aliBuild standard "#!/bin/bash -e": update the top-level
shebang in yaml-cpp.sh and the embedded script shebang (the second occurrence
currently at line 17) so both use "#!/bin/bash -e" to satisfy alidistlint's
ali:bad-shebang check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 43e44d3a-d18d-409d-86de-42add6558835
📒 Files selected for processing (2)
CHANGELOG.mdyaml-cpp.sh
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
The pinned 0.8.0 tarball fails to build on GCC 13+ / libstdc++ because src/emitterutils.cpp uses uint16_t and uint32_t without including <cstdint>; the transitive include path that previously provided it was removed in newer libstdc++ releases. Upstream 0.9.0 (4 Feb 2026) adds the missing #include directly. The new upstream tag is `yaml-cpp-0.9.0` (prefixed), unlike the bare `0.8.0` it replaces. Strip the `yaml-cpp-` prefix inside prefer_system_check so pkg-config --atleast-version still receives a clean semver. Keep `version: "%(tag_basename)s"` so the tag remains the single source of truth.
The pinned 0.8.0 tarball fails to build on GCC 13+ / libstdc++ because src/emitterutils.cpp uses uint16_t and uint32_t without including ; the transitive include path that previously provided it was removed in newer libstdc++ releases. Upstream 0.9.0 (4 Feb 2026) adds the missing #include directly.
The new upstream tag is
yaml-cpp-0.9.0(prefixed), unlike the bare0.8.0it replaces. Strip theyaml-cpp-prefix inside prefer_system_check so pkg-config --atleast-version still receives a clean semver. Keepversion: "%(tag_basename)s"so the tag remains the single source of truth.Related #234.
Summary by CodeRabbit
Bug Fixes
Chores