Use native line breaks in dfetch import and dfetch freeze manifests#909
Use native line breaks in dfetch import and dfetch freeze manifests#909
dfetch import and dfetch freeze manifests#909Conversation
WalkthroughThe changes modify how dfetch dumps manifest files by explicitly specifying native OS line separators via Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
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: 0
🧹 Nitpick comments (1)
dfetch/manifest/manifest.py (1)
331-336: Implementation correctly addresses issue #327.The addition of
line_break=os.linesepis supported in PyYAML 6.0.3 and will produce platform-native line endings as intended.However, be aware that this can cause version control conflicts if manifests are created on different platforms and committed without proper
.gitattributesconfiguration. Consider documenting that users should add*.yaml text eol=lfto their.gitattributesfile to normalize line endings in git, or accept that manifests may show as changed when switching platforms.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.rstdfetch/manifest/manifest.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build / build (windows-latest)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
dfetch/manifest/manifest.py (1)
419-425: Type annotation maintenance updates look good.The addition of
no-untyped-callto the ignore comments is appropriate for untyped library methods.CHANGELOG.rst (1)
29-29: Changelog entry is accurate; PR objectives contain outdated information.The changelog correctly lists only
dfetch freezeanddfetch import. Thedfetch initcommand does not callManifest.dump()—it simply copies a template file usingshutil.copyfile(). The PR objectives mentioningdfetch initappear to be incorrect or outdated and should be disregarded.
Fixes #327
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.