Conversation
- main.go: CLI 参数解析、信号处理 - config.go: 配置加载（.env + 环境变量） - docker.go: Docker Compose 服务发现与启停 - backup.go: Kopia 依赖检查、仓库连接、快照创建 - orchestrator.go: 主流程编排（停止→备份→恢复） - notify.go: Apprise 通知发送 - CLAUDE.md: Claude Code 项目指引文档 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- 新增 logger.go: 支持同时输出到终端和日志文件，可捕获日志内容供上传 - 新增 gist.go: 实现 GitHub Gist 日志上传和旧日志自动清理 - 修改 docker.go: 添加 StopParallel/StartParallel 方法实现并行服务管理 - 修改 orchestrator.go: 普通服务并行处理，优先服务保持顺序执行 - 修改 main.go: 集成新的日志初始化流程 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
kopia 会自己维护
- 新增 go/Makefile 支持多平台交叉编译 (linux/darwin/windows) - 新增 go-build-artifact.yml 用于 PR 和非 main 分支构建 - 新增 go-dev-release.yml 用于开发版自动发布 - 新增 go-prod-release.yml 用于 v2* tag 正式发布 - 更新 .gitignore 忽略 dist 目录 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- 说明两个版本的区别和版本号规则 (v1.x Shell, v2.x Go) - 添加 Go 版本的下载和使用说明 - 更新命令行参数表格，区分两版本支持的参数 - 添加 Go 版本的开发说明和构建命令 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a Go v2 implementation (CLI, orchestrator, Docker manager, Kopia backup, Gist uploader, Apprise notifier, logger), tests, Go build tooling (Makefile, go.mod), new GitHub Actions workflows for build/release, README/CLAUDE updates, and removes the prior dev-release workflow. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (main.go)
participant Orch as Orchestrator
participant Docker as DockerManager
participant Kopia as KopiaBackup
participant Lock as Lock (dir)
participant Gist as GistManager
participant Notifier as Notifier
CLI->>Orch: Run()
Orch->>Kopia: CheckDependencies()
Kopia-->>Orch: ok / err
Orch->>Kopia: CheckRepository()
Kopia-->>Orch: ok / err
Orch->>Lock: acquireLock()
Lock-->>Orch: acquired / fail
Orch->>Docker: DiscoverServices()
Docker-->>Orch: services
Orch->>Docker: ClassifyServices(priority list)
Docker-->>Orch: priority, normal
Orch->>Docker: StopParallel(normal)
Docker-->>Orch: results
Orch->>Docker: Stop(priority) (serial)
Docker-->>Orch: results
Orch->>Kopia: CreateSnapshot()
Kopia-->>Orch: snapshot result
Orch->>Docker: Start(priority) (serial)
Docker-->>Orch: results
Orch->>Docker: StartParallel(normal)
Docker-->>Orch: results
Orch->>Gist: Upload(logContent)
Gist-->>Orch: upload result
Orch->>Notifier: Send(final status)
Notifier-->>Orch: async ack
Orch->>Lock: releaseLock()
Lock-->>Orch: released
Orch-->>CLI: completion / exit
sequenceDiagram
participant Main as main.go
participant Config as Config loader
participant Logger as Logger
participant Orch as Orchestrator
Main->>Config: LoadConfig(path)
Config->>Config: load .env & parse env vars
Config-->>Main: *Config / error
Main->>Logger: InitLogger(cfg.LogFile)
Logger-->>Main: *os.File / nil
Main->>Main: cfg.Print()
Main->>Orch: NewOrchestrator(cfg, dryRun)
Main->>Orch: Run()
Orch-->>Main: result / error
Main->>Orch: Cleanup() on signal or exit
Orch-->>Main: done
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Summary of ChangesHello @YewFence, 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! 此拉取请求引入了YewResin Docker Compose备份工具的Go语言重构版本。新版本旨在提供更强大的跨平台兼容性、通过并行处理提升性能，并改进错误处理机制。它保留了停止服务、创建Kopia快照并重启服务的核心功能，同时通过模块化的Go代码库和增强的日志与通知系统，为用户带来了更稳定和高效的备份体验。 Highlights
Ignored Files
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
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c376a780dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
本次 PR 引入了 Go 语言对项目的重构，这是一个巨大的进步。整体代码结构清晰，模块化合理，并且充分利用了 Go 的并发特性来提升性能。新增的 Go 版本代码质量很高，包含了单元测试、跨平台编译支持和完善的错误处理机制。我的审查主要集中在一些可以提升代码健壮性和可维护性的细节上：
- 修正了从环境变量解析整数时的潜在 bug。
- 提高了布尔配置的容错性。
- 改进了交互式确认的输入处理方式。
- 指出了一个潜在的跨平台兼容性问题。
- 建议更新项目文档以反映 Go 版本的加入。
此外，go.mod中使用的 Go 版本（1.23）尚未正式发布，建议降级到稳定版本以方便其他开发者协作。总体而言，这是一次出色的重构，显著提升了项目的性能和可维护性。合并这些修改后，项目将更加完善。
| return false | ||
| } | ||
| // 在 Unix 系统上检查执行权限 | ||
| return info.Mode()&0111 != 0 |
There was a problem hiding this comment.
isExecutable 函数中 info.Mode()&0111 != 0 的检查方式在非 Unix 系统（如 Windows）上无法正常工作。由于项目提供了 Windows 平台的构建，这个函数可能会导致在 Windows 环境下无法正确识别可执行的 .sh 脚本（即使通过 Git Bash 或 WSL 运行）。考虑到跨平台兼容性，可以考虑调整这里的逻辑，或者明确在文档中指出在 Windows 上自定义启停脚本的限制。一个简单的改进是在 Windows 上总是返回 false，因为 .sh 脚本本身在原生 Windows 环境中不可执行。
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@CLAUDE.md`:
- Around line 1-76: Update CLAUDE.md to add a "Go v2" section describing the new
Go rewrite: document the go/ directory layout, list Go build/test commands
(e.g., make build, make all, make test under go/), outline the Go architecture
and key files (main.go, orchestrator.go, config.go, docker.go and any package
names), and enumerate Go-specific CI workflows (go-dev-release.yml,
go-prod-release.yml, go-build-artifact.yml); place this Section alongside the
existing shell v1 docs and include brief instructions on when to use the shell
vs Go implementation and how to build/test the Go binary.
In `@go/config.go`:
- Around line 147-154: getEnvInt currently uses fmt.Sscanf and returns the zero
value 0 when parsing fails (e.g., "abc"), so update getEnvInt to detect parse
errors and fall back to defaultVal; specifically, either replace fmt.Sscanf with
strconv.Atoi and if strconv.Atoi returns an error return defaultVal, or keep
Sscanf but check its return (number of items scanned or error) and return
defaultVal on failure; ensure the function still returns the parsed int when
successful and defaultVal otherwise.
In `@go/logger.go`:
- Around line 26-35: The LogCapture.Write method currently ignores errors from
each writer; update LogCapture.Write to collect and handle write errors from
lc.writers (and from appending to lc.buffer if applicable), returning a non-nil
error when writes fail (at minimum if all writers fail) and/or attempt a
fallback log to stderr; specifically modify func (lc *LogCapture) Write(p
[]byte) to iterate writers, capture individual errors, aggregate or pick the
first meaningful error, and return it (instead of always nil), and optionally
write a fallback message to os.Stderr when a writer.Write(p) returns an error.
- Around line 12-16: LogCapture's buffer is not synchronized and can be
written/read concurrently causing data races; add a sync.Mutex (or sync.RWMutex)
field to the LogCapture struct and protect accesses to buffer in Write and
GetContent (lock around appending to buffer and when returning its contents) and
also protect any iteration over writers while writing to them; update Write to
lock, append to buffer, unlock (or use defer), then write to writers under the
same lock or copy buffer to a local var for writing outside the lock, and update
GetContent to acquire the lock and return a copy of buffer to avoid exposing
internal slice.
In `@go/orchestrator.go`:
- Around line 143-149: The loop is appending full error strings to
failedServices; update handling so only service names are added—have
o.docker.StartParallel return structured results (e.g., a slice of structs with
ServiceName and Err) or a map from service name to error, then in the
error-handling branch iterate those results and append the ServiceName (not
err.Error()) to failedServices; adjust the loop that currently iterates over
errs from o.docker.StartParallel and replace err.Error() with the struct field
(e.g., result.ServiceName) or the map key so notifications list only service
names.
- Around line 44-126: The Run method acquires a lock via acquireLock() but never
releases it on the normal success path; change Run so releaseLock() is always
called after a successful acquire — simplest fix: call defer o.releaseLock()
immediately after o.acquireLock() returns nil (or ensure releaseLock() is
invoked in all return paths after services are stopped and snapshots/upload
complete). Update references in Run (acquireLock, releaseLock, startAllServices,
Cleanup) to avoid leaving the lock held.
🧹 Nitpick comments (15)
go/.gitignore (1)
1-2: Consider adding the binary name to ignore non-Windows builds.The
dist/directory will catch most build outputs, but if binaries are ever built outsidedist/(e.g., during local development withgo build), the Linux/macOS executables (without.exeextension) could be accidentally committed. Consider adding the binary name directly.Suggested additions
*.exe dist/ +# Local build output +yewresingo/logger_test.go (1)
11-27: Consider test isolation for global state.This test modifies the global
LogWriterandslogdefault logger. If other tests in this package also callInitLogger, they may interfere with each other when run in parallel (go test -parallel). Consider usingt.Cleanup()to reset the global state after each test, or restructure to avoid relying on globals in tests..github/workflows/go-dev-release.yml (1)
32-42: Pin the action version to avoid unexpected breakage.Using
@latestformarvinpinto/action-automatic-releasesis risky for CI stability. The static analysis also indicates this action's runner may be outdated. Consider pinning to a specific version or SHA, or migrating to a maintained alternative likesoftprops/action-gh-release.♻️ Suggested fix using softprops/action-gh-release
- name: Update Latest Release - uses: "marvinpinto/action-automatic-releases@latest" + uses: softprops/action-gh-release@v2 with: - repo_token: "${{ secrets.GITHUB_TOKEN }}" - automatic_release_tag: "latest" + tag_name: "latest" prerelease: true - title: "Development Build (Latest)" + name: "Development Build (Latest)" files: | yewresin.sh .env.example go/dist/*go/notify.go (1)
40-40: Fire-and-forget goroutine may not complete on application exit.The
go n.sendAsync(...)launches a detached goroutine. If the application exits shortly after callingSend()(e.g., at the end of a backup run), the notification may never be delivered. Consider using async.WaitGroupor channel-based mechanism to allow graceful waiting for pending notifications before shutdown.♻️ Example approach with WaitGroup
type Notifier struct { url string notifyURL string deviceName string wg sync.WaitGroup } func (n *Notifier) Send(title, body string) { // ... validation ... n.wg.Add(1) go func() { defer n.wg.Done() n.sendAsync(title, body) }() } func (n *Notifier) Wait() { n.wg.Wait() }Call
notifier.Wait()before application exit in the orchestrator.go/Makefile (2)
11-12: Consider addingCGO_ENABLED=0for fully static binaries.For maximum cross-platform compatibility and truly static binaries (especially for Linux), consider disabling CGO:
♻️ Suggested change
LDFLAGS := -s -w -X main.version=$(VERSION) -GO_BUILD := go build -ldflags "$(LDFLAGS)" +GO_BUILD := CGO_ENABLED=0 go build -ldflags "$(LDFLAGS)"This ensures binaries don't link against glibc and can run on any Linux distribution (e.g., Alpine, musl-based containers).
14-15: UnusedPLATFORMSvariable.The
PLATFORMSvariable is defined but never used. You could either remove it or leverage it for a loop-based build (though the current explicit targets are clearer for maintenance).go/config.go (1)
41-58: Consider environment variable isolation.
godotenv.Loadmodifies the process's global environment. This could cause issues ifLoadConfigis called multiple times or concurrently (e.g., in tests). For this CLI tool's single-run lifecycle, this is acceptable, but be aware of this behavior if the function is reused elsewhere.go/config_test.go (1)
21-31: Consider adding a test case for invalid integer input.Given the bug in
getEnvIntwhere invalid input returns 0 instead of the default, adding a test case liket.Setenv("TEST_INT", "invalid")would document the expected behavior (and currently fail to return the default).💡 Suggested test addition
// Test invalid input - should return default t.Setenv("TEST_INT", "invalid") if got := getEnvInt("TEST_INT", 7); got != 7 { t.Fatalf("expected default 7 for invalid input, got %d", got) }go/docker.go (1)
92-105: Consider adding a timeout to prevent hanging on unresponsive Docker daemon.
exec.Commandwithout a context or timeout could hang indefinitely if Docker is unresponsive.💡 Suggested improvement with context timeout
+import ( + "context" + "time" +) func (dm *DockerManager) IsRunning(svc *Service) bool { - cmd := exec.Command("docker", "compose", "ps", "-q") + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, "docker", "compose", "ps", "-q") cmd.Dir = svc.Path // ... }Apply similar timeout pattern to
StopandStartmethods.go/main.go (1)
67-75: Signal handler exits immediately after cleanup without allowing main goroutine to finish.The
os.Exit(1)in the signal handler prevents any deferred cleanup inmain()(likelogFile.Close()) from running. This is generally acceptable for signal handlers, but worth noting.💡 Alternative: Use context cancellation for graceful shutdown
A more sophisticated approach would use
context.Contextcancellation to signal shutdown toorch.Run(), allowing the main function to exit normally with deferred cleanup. However, the current approach is simpler and sufficient for this CLI tool.README.md (2)
206-221: Add language specifier to fenced code block.Per static analysis, this code block lacks a language specifier. Since this displays a directory structure, consider using
textorplaintext.Proposed fix
-``` +```text YewResin/ ├── yewresin.sh # 生成的脚本（由 make build 生成）
243-255: Add language specifier to fenced code block.Same issue as above - add
textorplaintextas the language specifier.Proposed fix
-``` +```text go/ ├── main.go # 程序入口，CLI 参数解析go/backup.go (1)
58-66: Substring matching may produce false positives.
strings.Contains(outputStr, k.expectedRemote)could match unintended paths. For example, ifexpectedRemoteisgdrive:backup, it would also matchgdrive:backup-oldin the output.Consider using a more precise match, such as parsing the status output or using a regex with word boundaries.
go/gist.go (1)
50-85: Potential timezone inconsistency in log timestamps.Line 51 uses
time.Now().UTC()for the filename timestamp, but Lines 84-85 usestartTime.Format(...)andtime.Now().Format(...)without explicit timezone handling. IfstartTimeis in local time (which it is, set in orchestrator.go Line 45), the log content will mix local time with UTC filename.Consider using consistent timezone handling throughout:
Proposed fix
- time.Now().Format("2006-01-02 15:04:05"), logContent) + time.Now().UTC().Format("2006-01-02 15:04:05 UTC"), logContent)Or convert
startTimeto UTC as well for consistency.go/orchestrator.go (1)
183-195: Useos.RemoveAll()for more robust lock cleanup.The lock directory is created empty with
os.Mkdir()andos.Remove()will successfully delete it. However, useos.RemoveAll()to be defensive against any future writes to the lock directory.Proposed fix
- if err := os.Remove(o.cfg.LockFile); err != nil { + if err := os.RemoveAll(o.cfg.LockFile); err != nil {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go/go.sumis excluded by!**/*.sum
📒 Files selected for processing (19)
.github/workflows/go-build-artifact.yml.github/workflows/go-dev-release.yml.github/workflows/go-prod-release.ymlCLAUDE.mdREADME.mdgo/.gitignorego/Makefilego/TODO.mdgo/backup.gogo/config.gogo/config_test.gogo/docker.gogo/gist.gogo/go.modgo/logger.gogo/logger_test.gogo/main.gogo/notify.gogo/orchestrator.go
🧰 Additional context used
🧬 Code graph analysis (3)
go/logger_test.go (1)
go/logger.go (2)
InitLogger(44-67)LogWriter(10-10)
go/main.go (3)
go/config.go (1)
LoadConfig(42-103)go/logger.go (1)
InitLogger(44-67)go/orchestrator.go (1)
NewOrchestrator(32-41)
go/config_test.go (1)
go/config.go (1)
LoadConfig(42-103)
🪛 actionlint (1.7.10)
.github/workflows/go-dev-release.yml
33-33: the runner of "marvinpinto/action-automatic-releases@latest" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 checkmake (0.2.2)
go/Makefile
[warning] 26-26: Target body for "all" exceeds allowed length of 5 (8).
(maxbodylength)
[warning] 82-82: Target body for "help" exceeds allowed length of 5 (18).
(maxbodylength)
🪛 markdownlint-cli2 (0.18.1)
README.md
206-206: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
243-243: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (38)
.github/workflows/go-build-artifact.yml (1)
1-44: Well-structured CI workflow.The workflow configuration is appropriate:
- Path filtering ensures it only runs for Go-related changes
- Go version (1.23) is consistent with
go.mod- Minimal permissions (
contents: read) follow the principle of least privilege- Latest action versions are used
.github/workflows/go-prod-release.yml (1)
1-38: Production release workflow looks solid.The workflow correctly:
- Triggers only on
v2*tags (appropriate for Go v2)- Passes the tag name as
VERSIONfor build-time embedding- Uses
softprops/action-gh-release@v2with auto-generated release notes- Has appropriate write permissions for release creation
go/TODO.md (1)
1-15: Good project tracking document.Clear checklist of implemented and pending features. The completed items align with the new files added in this PR (Gist upload, parallel operations, logging, Makefile, CI/CD, tests).
go/go.mod (1)
1-5: Module setup is correct.The
godotenvdependency at v1.5.1 is the latest available version with no known security vulnerabilities. The Go 1.23 version and minimal dependency footprint are appropriate for the project.go/logger_test.go (1)
29-49: LGTM!Good use of
t.TempDir()for automatic cleanup. The test correctly closes the file before reading to ensure all buffered data is flushed to disk..github/workflows/go-dev-release.yml (1)
39-42: Verify intentional inclusion of shell script in Go release.The release includes
yewresin.sh(shell v1) alongside Go binaries. If this is intentional for backward compatibility, consider documenting this. Otherwise, remove it to keep the Go release artifacts focused.go/notify.go (1)
44-68: LGTM on the HTTP client implementation.Good practices: timeout configured, response body closed via defer, appropriate status code validation, and error logging without failing the main flow.
go/Makefile (1)
19-100: LGTM on the overall Makefile structure.The build targets are well-organized with clear help documentation. The version embedding via ldflags and multi-platform support are properly implemented. The static analysis warnings about target body length are acceptable given the clarity and maintainability of the current structure.
go/config.go (4)
12-39: LGTM!The
Configstruct is well-organized with clear grouping of related fields and helpful Chinese comments explaining each field's purpose.
60-103: LGTM!Configuration loading logic is solid with proper validation of required fields, existence check for
BaseDir, and sensible defaults. Error messages are clear and actionable.
105-136: LGTM!The
GistToken,KopiaPassword,AppriseURL) while displaying useful configuration information. Good security practice.
167-172: LGTM!The
maskStringfunction provides reasonable masking logic—short strings are fully masked while longer strings show partial content for identification.go/config_test.go (5)
9-19: LGTM!Tests both fallback behavior and value override cases for
getEnvDefault.
33-57: LGTM!Good use of table-driven tests for boolean parsing with comprehensive coverage of common boolean representations.
59-68: LGTM!Tests both short string masking and partial masking for longer strings.
70-90: LGTM!Comprehensive test for successful config loading with proper use of
t.TempDir()andt.Setenv()for test isolation.
92-121: LGTM!Error condition tests properly verify that missing required fields and non-existent directories produce appropriate errors with Chinese error messages.
go/logger.go (1)
44-66: LGTM!The
InitLoggerfunction properly initializes the logging system with optional file output and configuresslogas the default logger. File handle is returned for proper cleanup by the caller.go/docker.go (7)
14-27: LGTM!Well-defined constants for compose filenames and a clear
Servicestruct with appropriate fields.
43-69: LGTM!Service discovery logic is straightforward—scans directories for compose files or scripts and checks running status.
107-148: LGTM!The
Stopmethod has clear priority logic for stop methods (scripts over compose commands), proper dry-run support, and good logging.
150-187: LGTM!The
Startmethod mirrorsStopwith appropriate script-first logic and dry-run support.
189-197: Note:isExecutableuses Unix-specific permission check.The
0111bitmask check works on Unix but may not behave as expected on Windows. Since this tool targets Docker environments (typically Linux), this is acceptable.
199-214: LGTM!
ClassifyServicescorrectly partitions services using case-insensitive matching for flexibility.
216-265: LGTM!Parallel operations are correctly implemented with
sync.WaitGroupfor coordination and mutex-protected error collection. The loop variable capture patterngo func(s *Service)is correct.go/main.go (4)
16-42: LGTM!CLI flag definitions are well-structured with both long and short forms, clear help text, and proper version handling.
44-65: LGTM!Configuration loading, logger initialization, and orchestrator creation follow a clear sequence with proper error handling and resource cleanup via
defer.
77-94: LGTM!Interactive confirmation flow is properly gated by flags, execution is timed, and errors are logged with appropriate exit codes.
97-114: LGTM!The
confirmfunction provides a clear warning banner and accepts common affirmative responses. Usingfmt.Scanlnis appropriate for this simple interactive prompt.go/backup.go (4)
11-25: LGTM!Clean struct definition and constructor with appropriate fields for Kopia backup management.
28-41: LGTM!Dependency checking with
exec.LookPathis the idiomatic Go approach for verifying CLI tool availability.
69-84: LGTM!Good practice passing the password via environment variable rather than command-line argument (avoids exposure in process listings).
87-105: LGTM!Proper dry-run handling and clear logging for snapshot creation.
go/gist.go (3)
16-35: LGTM!Good initialization with a sensible HTTP client timeout of 30 seconds.
139-200: LGTM!The cleanup logic correctly:
- Respects
keepFirstFileoption- Filters by log prefix to avoid deleting unrelated files
- Sorts lexicographically (works correctly with the timestamp format)
- Deletes oldest files first
108-110: Verify token type before changing authentication header format.GitHub's authentication header depends on the token type:
- Fine-grained PATs (prefix
github_pat_): UseAuthorization: Bearer <token>- Classic PATs (prefix
ghp_/gho_): UseAuthorization: token <token>Confirm which token type
g.tokenis before refactoring, as usingBearerwith a classic PAT will fail.go/orchestrator.go (2)
31-41: LGTM!Clean dependency injection pattern - all subsystems are initialized in the constructor.
163-181: LGTM!Using
os.Mkdiras an atomic lock is a good pattern for POSIX systems - the operation is atomic and will fail if the directory already exists.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@go/config.go`:
- Around line 43-59: LoadConfig currently ignores os.Stat errors and silently
proceeds when a caller passes an explicit configPath or when stat fails for
reasons other than "not exist" for the default path; update LoadConfig to
surface stat errors: call os.Stat(configPath) and if configPath != "" return the
stat error (wrap it) so missing/permission issues are reported, while if
configPath == "" treat only os.IsNotExist(err) as non-fatal and return other
errors (wrap and return) for the default path; only call
godotenv.Load(configPath) when stat succeeded. Reference LoadConfig and the
configPath handling logic.
♻️ Duplicate comments (1)
go/orchestrator.go (1)
144-151: Failed service list still uses error strings instead of names.
Line 150 appendserr.Error(), so notifications show full error text rather than clean service names. Consider returning structured results fromStartParallel(service name + error) and append the name only.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
go/config.gogo/logger.gogo/orchestrator.go
🚧 Files skipped from review as they are similar to previous changes (1)
- go/logger.go
🔇 Additional comments (6)
go/config.go (3)
13-40: Config struct is well‑scoped and readable.
Clear separation of required vs optional fields will make downstream usage straightforward.
106-137: Config.Print masks sensitive fields well.
Nice touch keeping tokens/passwords hidden while still showing key settings.
141-174: Env helper functions look solid.
Concise defaults and parsing keep config loading predictable.go/orchestrator.go (3)
11-40: Orchestrator initialization is cohesive.
Dependency wiring is clear and centralized.
44-129: Run flow now cleans up locks and restores services on failure.
Thedeferat Line 61 plus restart on failure at Line 87 improves resilience.
159-198: Cleanup/lock handling is straightforward and idempotent.
releaseLockguards onlockAcquiredand logs failures, keeping cleanup safe.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@go/config_test.go`:
- Around line 3-7: Tests currently pass a missing config path so LoadConfig
fails early with the "配置文件不存在或不可访问" error; modify the tests that call LoadConfig
in config_test.go (and the similar blocks around lines 75-123) to create an
empty temporary .env file (or call LoadConfig("") to use defaults) before
invoking LoadConfig, e.g. create a temp file via os.CreateTemp, write nothing,
close it, and pass its path into LoadConfig so the function proceeds to
environment validation; ensure the temp file is removed after the test to avoid
leaks.
In `@go/docker.go`:
- Around line 50-71: DiscoverServices currently ignores failures from IsRunning
and treats any error as "not running"; change IsRunning on DockerManager to
return (bool, error) and update DiscoverServices to call running, err :=
dm.IsRunning(svc) and if err != nil return nil, fmt.Errorf("检查服务运行状态失败 for %s:
%w", svc.Name, err) (or similar) instead of silently setting svc.Running=false;
set svc.Running = running only on success. Update all other IsRunning call sites
(e.g., Stop and any other usages) to handle the (bool, error) return and
propagate or handle errors accordingly so orchestration fails fast on docker
command failures.
In `@README.md`:
- Around line 207-208: Add explicit language identifiers to the fenced code
blocks that currently lack them: update the fences containing the literal lines
"YewResin/" and "go/" (and the similar fences at lines noted 244-245) to use
```text so markdownlint MD040 is satisfied; locate the triple-backtick fences
that immediately surround the strings "YewResin/" and "go/" in README.md and
change their opening fences to ```text.
♻️ Duplicate comments (1)
go/orchestrator.go (1)
145-152: Return service names instead of raw error strings in restore summary.
failedServicescurrently appendserr.Error(), which produces noisy notifications. Prefer structured results fromStartParallelso you can append the service name only.♻️ Suggested shape (requires StartParallel to return service-aware results)
-if errs := o.docker.StartParallel(o.normalServices); len(errs) > 0 { - for _, err := range errs { - slog.Error("启动服务失败", "error", err) - // 从错误信息中提取服务名 - failedServices = append(failedServices, err.Error()) - } -} +if results := o.docker.StartParallel(o.normalServices); len(results) > 0 { + for _, r := range results { + slog.Error("启动服务失败", "service", r.Service.Name, "error", r.Err) + failedServices = append(failedServices, r.Service.Name) + } +}
🧹 Nitpick comments (1)
go/backup.go (1)
58-63: Accept both quoted and unquoted remote paths fromkopia repository status.The current check only passes if the output includes quotes. Some Kopia versions may output the remote unquoted, which would incorrectly fail the repository check. Consider accepting both formats and verify against your target Kopia version.
♻️ Proposed robustness tweak
outputStr := string(output) quotedExpected := `"` + k.expectedRemote + `"` -if !strings.Contains(outputStr, quotedExpected) { +if !strings.Contains(outputStr, quotedExpected) && !strings.Contains(outputStr, k.expectedRemote) { return fmt.Errorf("Kopia 仓库路径不匹配，期望: %s", k.expectedRemote) }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/dev-release.ymlREADME.mdgo/.gitignorego/Makefilego/backup.gogo/config.gogo/config_test.gogo/docker.gogo/notify.gogo/orchestrator.go
💤 Files with no reviewable changes (1)
- .github/workflows/dev-release.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- go/notify.go
🧰 Additional context used
🧬 Code graph analysis (2)
go/config_test.go (1)
go/config.go (1)
LoadConfig(44-115)
go/orchestrator.go (6)
go/config.go (1)
Config(14-41)go/docker.go (4)
DockerManager(32-36)Service(25-29)NewDockerManager(39-48)ClassifyServices(219-233)go/backup.go (2)
KopiaBackup(12-16)NewKopiaBackup(19-25)go/notify.go (2)
Notifier(14-19)NewNotifier(22-28)go/gist.go (2)
GistManager(16-23)NewGistManager(26-35)go/logger.go (1)
LogWriter(11-11)
🪛 checkmake (0.2.2)
go/Makefile
[warning] 26-26: Target body for "all" exceeds allowed length of 5 (8).
(maxbodylength)
[warning] 82-82: Target body for "help" exceeds allowed length of 5 (18).
(maxbodylength)
🪛 GitHub Actions: Go Build Artifact
go/config_test.go
[error] 83-83: TestLoadConfigSuccess failed: unexpected error: 配置文件不存在或不可访问: stat /tmp/TestLoadConfigSuccess3780344504/002/missing.env: no such file or directory. Command failed: go test -v ./... (exit code 1)
[error] 103-103: TestLoadConfigMissingBaseDir failed: expected BASE_DIR error, got 配置文件不存在或不可访问: stat /tmp/TestLoadConfigMissingBaseDir1994894384/001/missing.env: no such file or directory. Command failed: go test -v ./... (exit code 1)
[error] 114-114: TestLoadConfigMissingExpectedRemote failed: expected EXPECTED_REMOTE error, got 配置文件不存在或不可访问: stat /tmp/TestLoadConfigMissingExpectedRemote667949831/002/missing.env: no such file or directory. Command failed: go test -v ./... (exit code 1)
[error] 124-124: TestLoadConfigBaseDirNotExist failed: expected BASE_DIR not exist error, got 配置文件不存在或不可访问: stat /tmp/TestLoadConfigBaseDirNotExist4091979593/002/missing.env: no such file or directory. Command failed: go test -v ./... (exit code 1)
🪛 markdownlint-cli2 (0.18.1)
README.md
207-207: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
244-244: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (11)
go/.gitignore (1)
1-4: LGTM: build outputs are ignored correctly.These patterns align with the binary and dist artifacts produced by the Makefile/CI.
go/Makefile (1)
7-12: No issues found. Themain.versionvariable exists ingo/main.go(line 14) as a mutable string variable, so the-X main.version=$(VERSION)ldflags injection will work correctly.go/config.go (4)
13-41: Config schema is clear and well-grouped.
43-115: LoadConfig flow is robust for default vs. explicit paths.
117-149: Print output masks secrets while keeping useful context.Also applies to: 177-186
153-175: Env helpers are straightforward and safe.go/orchestrator.go (5)
31-40: Constructor wiring looks consistent.
44-129: Run flow is cohesive and ensures lock release.
160-166: Cleanup performs best‑effort recovery and releases the lock.
168-186: Directory lock acquisition logic looks solid.
188-200: Lock release resets state correctly.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
/gemini review |
go/orchestrator.go
Outdated
| for _, err := range errs { | ||
| slog.Error("启动服务失败", "error", err) | ||
| // 从错误信息中提取服务名 | ||
| failedServices = append(failedServices, err.Error()) |
There was a problem hiding this comment.
这里将完整的错误信息添加到了 failedServices 数组中，这可能导致最终的通知信息比较冗长（例如：[service-name: start failed: ...]）。如注释所说，如果只想提取服务名，可以考虑从错误字符串中解析。一个更健壮的方式是使用 strings.SplitN(err.Error(), ":", 2)[0] 来获取服务名，这样可以避免当错误信息本身包含冒号时出现问题。
| failedServices = append(failedServices, err.Error()) | |
| failedServices = append(failedServices, strings.SplitN(err.Error(), ":", 2)[0]) |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@README.md`:
- Line 146: Update the environment variables table to include the same "Shell"
and "Go" support columns used by the command-line parameters table; locate the
env vars table and add two new columns labeled "Shell" and "Go", then update
each variable row (including DOCKER_COMMAND_TIMEOUT_SECONDS) to mark support —
for DOCKER_COMMAND_TIMEOUT_SECONDS set Shell to "-" and Go to "✓" — follow the
existing column alignment and formatting used by the command-line parameters
table to keep consistency.
♻️ Duplicate comments (3)
go/config.go (2)
160-167: Previous review concern has been addressed.
getEnvIntnow correctly usesstrconv.Atoiwith proper error handling, returningdefaultValwhen parsing fails instead of silently returning 0.
169-175: Previous review concern has been addressed.
getEnvBoolnow correctly converts the value to lowercase before comparison, making it case-insensitive for values like "TRUE", "YES", "True", etc.README.md (1)
207-208: Add language identifiers to fenced code blocks.The code blocks at lines 207 and 244 are missing language identifiers, which causes markdownlint violations and inconsistent rendering. This issue was flagged in a previous review but only partially addressed.
♻️ Proposed fix
-``` +```text YewResin/-``` +```text go/Based on static analysis hints and past review comments.
Also applies to: 244-245
🧹 Nitpick comments (2)
README.md (2)
494-496: Remove excessive blank lines at end of file.Standard practice is to have exactly one blank line at the end of a markdown file, not three.
♻️ Proposed fix
MIT - +
5-9: Consider clarifying the relationship between "identical core functionality" and "better performance."The introduction states both that the Go version offers "better performance and parallel processing" (line 7) and that "core functionality is completely identical" (line 9). While technically accurate (same outcomes, different implementation efficiency), this might be slightly confusing to readers.
Consider adding a brief clarification, such as:
两个版本的核心功能完全一致，依赖也相同，可根据使用场景自由选择。Go 版本在实现上更高效，但对用户而言功能表现一致。
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/build-artifact.yml:
- Around line 3-8: Update the pull_request workflow's paths filter so changes to
build-critical files trigger the job: modify the existing paths list under the
pull_request trigger (the paths: key currently containing 'src/**') to include
'Makefile' and '.env.example' as additional entries so updates to the Makefile
and the .env.example artifact will run the workflow.
♻️ Duplicate comments (1)
go/docker.go (1)
229-240: Windows.shexecutability check will cause false positives.On native Windows,
./compose-up.shwon’t run unless a shell is invoked explicitly, so marking.shas executable can break stop/start even when a compose file exists. Consider returning false on Windows (or invokingbashexplicitly / adding.cmdsupport) to avoid hard failures and allow the compose fallback.💡 Suggested fix (prefer fallback to docker compose on Windows)
func (dm *DockerManager) isExecutable(path string) bool { info, err := os.Stat(path) if err != nil { return false } if runtime.GOOS == "windows" { - return !info.IsDir() && strings.EqualFold(filepath.Ext(path), ".sh") + // .sh isn't directly executable on native Windows; fall back to docker compose. + return false } // 在 Unix 系统上检查执行权限 return info.Mode()&0111 != 0 }
🧹 Nitpick comments (1)
.github/workflows/go-build-artifact.yml (1)
37-41: Prefergo-version-fileto keep the Go version in sync.Hard-coding Go 1.23 can drift from the
go/go.modgodirective over time. Consider usinggo-version-fileso the workflow always matches the module version.♻️ Proposed change
- with: - go-version: '1.23' + with: + go-version-file: 'go/go.mod'
增加 docker 依赖检查
|
/gemini review |
YewFence
force-pushed
the
feature/refactor-go
branch
from
ad2628e to
79f6c2f
Compare
There was a problem hiding this comment.
Code Review
这个 Pull Request 主要是用 Go 语言重构了原有的 Shell 脚本项目，实现了跨平台二进制文件的构建。整体代码结构清晰，模块化做得很好，并且充分利用了 Go 的并发特性来提升服务启停的效率。
我的评审主要集中在以下几个方面：
- 健壮性: 建议在与外部工具（如
kopia）交互时使用 JSON 格式，以提高稳定性。 - 资源管理: 优化了网络请求中
http.Client的使用方式，避免不必要的资源消耗。 - 一致性: 统一了代码中时间戳的处理方式，全部采用 UTC 时间，以避免时区问题。
此外，代码中对并发、错误处理和信号处理的实现都非常出色。文档更新也很详尽，清晰地说明了新旧两个版本的使用方式。总体来说，这是一次高质量的重构。
|
@CodeRabbit summary |
✅ Actions performedSummary regeneration triggered. |
1 participant
可以编译成多平台单可执行文件，错误处理更完善
顺便修改了一下 Github Action，删除了原来的 latest 版本发布，手动发布更明确点
增加了手动触发 Github Action 的功能，去除了 Push 即构建的功能
Summary by CodeRabbit
New Features
Build
CI
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.