Conversation
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 primarily focuses on a significant dependency upgrade, migrating the project to Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
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
|
![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 主要将 funk 库升级到 v2，并为 Agent 集成引入了新的 skills 命令。大部分变动是依赖更新和针对新版 funk 的 API 调整。然而，我在 cmd/webcmd/server.go 文件中发现了一些关于错误处理的问题。具体来说，有几处不恰当地使用了 assert.Must，可能导致应用崩溃或行为不正确。在 handleCommand 函数中存在一个严重 bug，命令执行失败将导致 panic 而不是被优雅地处理。此外，filepath.Walk 的错误被忽略，这可能会掩盖潜在的文件系统问题。
|
|
||
| output, err := cmd.CombinedOutput() | ||
|
|
||
| assert.Must(err) |
There was a problem hiding this comment.
这是一个 bug。assert.Must(err) 会在 cmd.CombinedOutput() 返回错误时导致 panic。这会阻止后续的错误处理逻辑（例如设置 result.Success = false 和 result.Error）执行。因此，客户端将永远不会收到关于命令失败的正确响应。请移除此行，让后续代码来处理错误。
| go func() { | ||
| time.Sleep(500 * time.Millisecond) | ||
| openBrowser(url) | ||
| assert.Must(openBrowser(url)) | ||
| }() |
There was a problem hiding this comment.
在 goroutine 中使用 assert.Must 可能会导致整个应用程序崩溃。openBrowser 失败（例如，在没有图形界面的环境中）是一个非关键性错误，不应该导致服务停止。建议在这里记录错误而不是 panic。
| go func() { | |
| time.Sleep(500 * time.Millisecond) | |
| openBrowser(url) | |
| assert.Must(openBrowser(url)) | |
| }() | |
| go func() { | |
| time.Sleep(500 * time.Millisecond) | |
| if err := openBrowser(url); err != nil { | |
| slog.Warn("failed to open browser", "url", url, "err", err) | |
| } | |
| }() |
| go func() { | ||
| <-ctx.Done() | ||
| shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) | ||
| defer cancel() | ||
| s.server.Shutdown(shutdownCtx) | ||
| assert.Must(s.server.Shutdown(shutdownCtx)) | ||
| }() |
There was a problem hiding this comment.
在 goroutine 中对 s.server.Shutdown 使用 assert.Must 同样有风险。如果 Shutdown 返回错误，将导致整个程序 panic，这对于优雅停机来说是不理想的。建议记录错误而不是 panic。
| go func() { | |
| <-ctx.Done() | |
| shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) | |
| defer cancel() | |
| s.server.Shutdown(shutdownCtx) | |
| assert.Must(s.server.Shutdown(shutdownCtx)) | |
| }() | |
| go func() { | |
| <-ctx.Done() | |
| shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) | |
| defer cancel() | |
| if err := s.server.Shutdown(shutdownCtx); err != nil { | |
| slog.Error("server shutdown failed", "err", err) | |
| } | |
| }() |
| filepath.Walk(rootPath, func(path string, info fs.FileInfo, err error) error { | ||
| assert.Must(filepath.Walk(rootPath, func(path string, info fs.FileInfo, err error) error { | ||
| if err != nil { | ||
| return nil |
There was a problem hiding this comment.
在 filepath.Walk 的回调中，当 err != nil 时返回 nil 会导致错误被忽略。这可能会掩盖文件系统问题（例如权限错误），导致函数静默失败或返回不完整的结果。应该将错误向上传播。
| return nil | |
| return err |
| filepath.Walk(rootPath, func(path string, info fs.FileInfo, err error) error { | ||
| assert.Must(filepath.Walk(rootPath, func(path string, info fs.FileInfo, err error) error { | ||
| if err != nil { | ||
| return nil |
There was a problem hiding this comment.
在 filepath.Walk 的回调中，当 err != nil 时返回 nil 会导致错误被忽略。这可能会掩盖文件系统问题（例如权限错误），导致函数静默失败或返回不完整的结果。应该将错误向上传播。
| return nil | |
| return err |
| filepath.Walk(vendorPath, func(path string, info fs.FileInfo, err error) error { | ||
| assert.Must(filepath.Walk(vendorPath, func(path string, info fs.FileInfo, err error) error { | ||
| if err != nil { | ||
| return nil |
There was a problem hiding this comment.
在 filepath.Walk 的回调中，当 err != nil 时返回 nil 会导致错误被忽略。这可能会掩盖文件系统问题（例如权限错误），导致函数静默失败或返回不完整的结果。应该将错误向上传播。
| return nil | |
| return err |
There was a problem hiding this comment.
Pull request overview
This PR is an “upgrade” pass that primarily updates documentation/formatting, adds Agent Skills scaffolding support, and extends the formatter to support an additional engine.
Changes:
- Add a
protobuild skillscommand to generate an Agent Skills template (optional OpenAI metadata). - Extend
protobuild formatwith a--clang-formatengine and--clang-styleoption. - Restructure/translate multiple docs (Mermaid diagrams, tables) and remove legacy Gemini/Chinese docs files.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
proto/example/v1/user.proto |
Reformat retag.tags option blocks into a more compact style. |
docs/MULTI_SOURCE_DEPS.md |
Rewrite and reorganize the multi-source dependency design doc; switch diagrams to Mermaid. |
docs/EXAMPLES.md |
Adjust headings/anchors and table formatting; remove bilingual headers. |
docs/DESIGN_CN.md |
Remove the separate Chinese design doc (content appears consolidated elsewhere). |
docs/DESIGN.md |
Convert the design doc content to Chinese and replace ASCII diagrams with Mermaid. |
docs/AUDIT_REVIEW.md |
Table formatting and layout normalization. |
docs/AGENT_SKILL.md |
Add a guide/template for exposing protobuild as an Agent Skill. |
cmd/protobuild/cmd.go |
Add skills subcommand that writes .agents/skills/protobuild templates. |
cmd/formatcmd/cmd.go |
Add clang-format formatting path, file collection, and unified diff output. |
README_CN.md |
Remove the standalone Chinese README. |
README.md |
Rewrite README content in Chinese; document new clang-format options and add Mermaid architecture diagram. |
.github/gemini.json |
Remove Gemini Code Assist config. |
.github/gemini-instructions.md |
Remove Gemini review instructions. |
.github/copilot-instructions.md |
Update link to the code review guide location under .github/. |
.github/codereview/CODE_REVIEW_GUIDE_CN.md |
Add a consolidated Chinese code review guide/checklist. |
You can also share your feedback on Copilot code review. Take the survey.
| Use: name, | ||
| Short: "Format Protobuf files using buf format", | ||
| Short: "Format Protobuf files (buf, builtin, or clang-format)", | ||
| Long: `Format Protobuf files using buf format command. | ||
|
|
||
| By default, this command formats files in the 'root' directories defined in protobuf.yaml. |
| tmp, err := os.CreateTemp("", "protobuild-clang-format-*.proto") | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| tmpPath := tmp.Name() | ||
| defer os.Remove(tmpPath) | ||
|
|
||
| if _, err := tmp.Write(formatted); err != nil { | ||
| _ = tmp.Close() | ||
| return "", err | ||
| } | ||
| if err := tmp.Close(); err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| cmd := exec.Command("diff", "-u", "-L", filePath, "-L", filePath+" (clang-format)", filePath, tmpPath) |
| # 使用 clang-format（默认 style=file） | ||
| protobuild format --clang-format -w | ||
|
|
||
| # 使用 clang-format 并指定 style | ||
| protobuild format --clang-format --clang-style google -w |
| author: pubgo | ||
| version: "1.0.0" |
| defaultOpenAIContent = `interface: | ||
| display_name: "Protobuild" | ||
| short_description: "Proto build/lint/format tool" | ||
| icon_small: "./assets/protobuild-16.png" | ||
| icon_large: "./assets/protobuild-64.png" | ||
| brand_color: "#0F9D58" | ||
| default_prompt: "Use protobuild to vendor deps and generate proto code." |
| } | ||
|
|
||
| func normalizeTemplateIndent(content string) string { | ||
| return strings.ReplaceAll(content, "\n\t", "\n") |
Description / 描述
Type of Change / 变更类型
Related Issues / 关联 Issue
Pre-submission Checklist / 提交前检查清单
Critical Issues / 关键问题 🔴
[REQ]/ 无需求不匹配[LOGI]/ 无逻辑缺陷[SEC]/ 无安全漏洞[AUTH]/ 无认证授权问题High Priority / 高优先级 🟠
[DSN]/ 设计合理[RBST]/ 错误处理健壮[TRANS]/ 事务处理正确[CONC]/ 无并发问题[PERF]/ 性能可接受Code Quality / 代码质量 🟡🟢
[MAIN]/ 代码可维护[CPL]/ 无不必要耦合[READ]/ 代码可读[DUP]/ 无代码重复[NAM]/ 命名清晰Testing / 测试
Documentation / 文档
Screenshots / 截图
Additional Notes / 补充说明