Conversation
Summary of ChangesHello @kooksee, 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 represents a major overhaul and rebranding of the project, transitioning from 'SwaGin' to 'OpenDoc'. The core focus is on enhancing the Go OpenAPI documentation generation capabilities, making it more robust, maintainable, and aligned with modern Go practices. Key changes include the integration of 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. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
| lo.Must0(os.WriteFile("./internal/examples/openapi.yaml", | ||
| lo.Must(yaml.Marshal(lo.Must(doc.BuildSwagger().MarshalYAML()))), 0644)) |
There was a problem hiding this comment.
你好，这部分写入 openapi.yaml 文件的逻辑似乎有问题。
doc.MarshalYAML() 会返回一个 []byte 类型的 YAML 数据。你再对这个 []byte 使用 yaml.Marshal，会导致对一个已经是 YAML 格式的字节切片再次进行 YAML 编码，结果通常是一个 Base64 编码的字符串，而不是预期的 YAML 文件内容。
之前的写法是正确的。我建议恢复之前的写法，或者使用更简洁的方式。这个 bug 会导致生成的 openapi.yaml 文件内容不正确，从而影响文档的可用性。
| lo.Must0(os.WriteFile("./internal/examples/openapi.yaml", | |
| lo.Must(yaml.Marshal(lo.Must(doc.BuildSwagger().MarshalYAML()))), 0644)) | |
| data, err := doc.MarshalYAML() | |
| lo.Must0(err) | |
| lo.Must0(os.WriteFile("./internal/examples/openapi.yaml", data, 0644)) |
| getTag(tags, queryTag, func(tag *structtag.Tag) { | ||
| parameter = openapi3.NewQueryParameter(tag.Name) | ||
| if !tag.HasOption(omitempty) { | ||
| // Check required tag first, then fallback to omitempty | ||
| requiredTag, _ := tags.Get(required) | ||
| if requiredTag != nil && requiredTag.Name == "true" { | ||
| parameter.Required = true | ||
| } else if !tag.HasOption(omitempty) { | ||
| parameter.Required = true | ||
| } | ||
| }) | ||
|
|
||
| getTag(tags, headerTag, func(tag *structtag.Tag) { | ||
| parameter = openapi3.NewHeaderParameter(tag.Name) | ||
| if !tag.HasOption(omitempty) { | ||
| // Check required tag first, then fallback to omitempty | ||
| requiredTag, _ := tags.Get(required) | ||
| if requiredTag != nil && requiredTag.Name == "true" { | ||
| parameter.Required = true | ||
| } else if !tag.HasOption(omitempty) { | ||
| parameter.Required = true | ||
| } | ||
| }) | ||
|
|
||
| getTag(tags, cookieTag, func(tag *structtag.Tag) { | ||
| parameter = openapi3.NewCookieParameter(tag.Name) | ||
| if !tag.HasOption(omitempty) { | ||
| // Check required tag first, then fallback to omitempty | ||
| requiredTag, _ := tags.Get(required) | ||
| if requiredTag != nil && requiredTag.Name == "true" { | ||
| parameter.Required = true | ||
| } else if !tag.HasOption(omitempty) { | ||
| parameter.Required = true | ||
| } | ||
| }) |
There was a problem hiding this comment.
你好，这部分用于判断参数是否必需（required）的逻辑存在一些问题：
- 逻辑重复：
queryTag,headerTag,cookieTag的处理逻辑几乎完全一样，导致了代码重复。建议将这部分逻辑提取到一个辅助函数中。 - 逻辑错误：
else if !tag.HasOption(omitempty)这部分判断是错误的。这里的tag是query,header或cookie标签，它们通常不像json标签那样有omitempty选项。这个逻辑可能是从处理json标签的代码中复制过来的，但在这里不适用，可能会导致参数被错误地标记为必需。
参数是否必需的正确逻辑应该是：
- 路径参数（
path或uri）总是必需的。 - 其他类型的参数（
query,header,cookie）当且仅当它们有required:"true"标签时才是必需的。
我建议移除 !tag.HasOption(omitempty) 的判断，并重构这部分代码以消除重复。这个错误可能会导致 API 规范与实际行为不符，影响客户端的生成和使用。
| enable: | ||
| - govet |
There was a problem hiding this comment.
你好，感谢你添加了 golangci-lint 的支持！这是一个很好的改进。
不过，目前只启用了 govet 这一个 linter。为了更好地保证代码质量和发现潜在问题，我建议启用更多有用的 linter。例如：
staticcheck: 一个强大的静态分析工具集，可以发现很多细微的 bug。unused: 检查未使用的代码。ineffassign: 检查未使用的变量赋值。errcheck: 检查未处理的 error 返回值。gofumpt: 确保代码格式一致。
启用这些 linter 可以帮助我们更早地发现问题，提升代码的健壮性。
enable:
- govet
- staticcheck
- unused
- ineffassign
- errcheck
- gofumpt| getTag(tags, required, func(_ *structtag.Tag) { | ||
| if _t, err := tags.Get(required); err == nil && _t.Name == "true" { | ||
| // Mark as required by removing AllowEmptyValue | ||
| fieldSchema.AllowEmptyValue = false | ||
| } | ||
| }) |
There was a problem hiding this comment.
你好，这部分处理 required 标签的逻辑可以简化。
getTag 的回调函数只有在找到标签时才会执行，所以在回调内部再次调用 tags.Get(required) 是多余的。可以直接使用回调函数传入的 tag 参数，这样代码会更简洁易读。
| getTag(tags, required, func(_ *structtag.Tag) { | |
| if _t, err := tags.Get(required); err == nil && _t.Name == "true" { | |
| // Mark as required by removing AllowEmptyValue | |
| fieldSchema.AllowEmptyValue = false | |
| } | |
| }) | |
| getTag(tags, required, func(tag *structtag.Tag) { | |
| if tag.Name == "true" { | |
| // Mark as required by removing AllowEmptyValue | |
| fieldSchema.AllowEmptyValue = false | |
| } | |
| }) |
1 participant
No description provided.