fix: Windows path compat, move types to devDeps, parameterize SQL#26
Merged
fix: Windows path compat, move types to devDeps, parameterize SQL#26
Conversation
leaperone-bot
approved these changes
Mar 31, 2026
leaperone-bot
left a comment
There was a problem hiding this comment.
🤖 Code Review 🟢
总体评价
这是一个高质量的小修复 PR,三个改动都是正确且有意义的。
✅ Windows 路径兼容 (config-manager.ts)
将 this.configPath.substring(0, this.configPath.lastIndexOf('/')) 替换为 dirname(this.configPath) 是正确的做法。lastIndexOf('/') 在 Windows 上无法处理 \ 分隔符,而 dirname() 是跨平台的标准方案。
✅ 类型包移至 devDependencies (package.json)
@types/better-sqlite3 是纯类型定义包,仅在编译时使用,移至 devDependencies 是正确的分类。
✅ SQL 参数化查询 (db.ts)
将模板字符串拼接 VALUES (${DatabaseManager.SCHEMA_VERSION}) 改为参数化查询 VALUES (?) + .run(...) 是更安全的写法。虽然此处 SCHEMA_VERSION 是内部常量(非用户输入),不存在实际的 SQL 注入风险,但保持一致的参数化风格是好的编码习惯,也与文件中其他查询的风格保持一致。
📝 小建议(非阻塞)
- 注意到
main分支上的db.ts仍然是旧代码(使用模板字符串),说明此 PR 确实解决了问题。建议后续考虑为ConfigManager.save()和DatabaseManager的初始化逻辑增加单元测试,覆盖 Windows 路径场景和 schema 版本升级场景。 pnpm-lock.yaml未在 diff 中出现。如果移动依赖后未重新生成 lockfile,可能导致 CI 安装不一致。建议确认 lockfile 是否已同步更新。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #15, closes #16, closes #17
Summary
lastIndexOf('/')withdirname()in config-manager for Windows compatibility@types/better-sqlite3from dependencies to devDependencies