Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

feat(goctl): update mongo generate error #5123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jaronnie wants to merge 1 commit into zeromicro:master
base: master
Choose a base branch
Loading
from jzero-io:goctl_mongo_generate

Conversation

Copy link
Contributor

@jaronnie jaronnie commented Aug 27, 2025

No description provided.

Copy link

codecov bot commented Aug 27, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@kevwan kevwan requested a review from Copilot September 1, 2025 13:27
@kevwan kevwan self-assigned this Sep 1, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR updates the generateError function in the MongoDB model generator to iterate over all types in the context and generate error templates for each type individually, rather than using the entire context as template data.

  • Changes the error generation to loop through each type in ctx.Types
  • Updates template data to use individual type names with title casing
  • Replaces direct context usage with type-specific template variables

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +126 to 131
for _, t := range ctx.Types {
text, err := pathx.LoadTemplate(category, errTemplateFile, template.Error)
if err != nil {
return err
}

Copy link

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The template is being loaded inside the loop for each type, which is inefficient. Consider loading the template once before the loop to avoid repeated I/O operations.

Suggested change
for_, t := range ctx.Types {
text, err :=pathx.LoadTemplate(category, errTemplateFile, template.Error)
if err!=nil {
returnerr
}
text, err := pathx.LoadTemplate(category, errTemplateFile, template.Error)
if err !=nil {
return err
}
for_, t:=range ctx.Types {

Copilot uses AI. Check for mistakes.

Comment on lines +126 to 139
for _, t := range ctx.Types {
text, err := pathx.LoadTemplate(category, errTemplateFile, template.Error)
if err != nil {
return err
}

output := filepath.Join(ctx.Output, "error.go")
output := filepath.Join(ctx.Output, "error.go")
if err = util.With("error").Parse(text).GoFmt(true).SaveTo(map[string]any{
"Type": stringx.From(t).Title(),
}, output, false); err != nil {
return err
}
}

Copy link

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same output file 'error.go' is being overwritten in each iteration of the loop. This means only the last type's error template will be preserved, and previous types' error definitions will be lost.

Suggested change
for _, t := range ctx.Types {
text, err := pathx.LoadTemplate(category, errTemplateFile, template.Error)
if err != nil {
return err
}
output := filepath.Join(ctx.Output, "error.go")
output := filepath.Join(ctx.Output, "error.go")
if err = util.With("error").Parse(text).GoFmt(true).SaveTo(map[string]any{
"Type": stringx.From(t).Title(),
}, output, false); err != nil {
return err
}
}
var errorDefs []string
for _, t := range ctx.Types {
text, err := pathx.LoadTemplate(category, errTemplateFile, template.Error)
if err != nil {
return err
}
rendered, err := util.With("error").Parse(text).GoFmt(true).Build(map[string]any{
"Type": stringx.From(t).Title(),
})
if err != nil {
return err
}
errorDefs = append(errorDefs, rendered)
}
output := filepath.Join(ctx.Output, "error.go")
combined := ""
for _, def := range errorDefs {
combined += def + "\n"
}
if err := pathx.WriteFile(output, combined, false); err != nil {
return err
}

Copilot uses AI. Check for mistakes.

output := filepath.Join(ctx.Output, "error.go")
output := filepath.Join(ctx.Output, "error.go")
if err = util.With("error").Parse(text).GoFmt(true).SaveTo(map[string]any{
"Type": stringx.From(t).Title(),
Copy link
Collaborator

@kesonan kesonan Sep 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you want to inject a Type variable into the template? I don't see that the template uses this variable. What problem are you trying to solve?

@kesonan kesonan self-requested a review September 6, 2025 01:02
@kesonan kesonan added the do-not-merge/hold Indicates that a PR should not merge because of known issues or need more discussion. label Sep 6, 2025
Copy link
Contributor Author

jaronnie commented Sep 6, 2025

I want to customize the template. The default package is model. I want to add more build-in functions and a new type variable to customize the template package.

Copy link
Contributor Author

jaronnie commented Sep 6, 2025 •
edited
Loading

e.g.

package {{.Type | lower}}
import (
	"errors"
	"github.com/zeromicro/go-zero/core/stores/mon"
)
var (
	ErrNotFound = mon.ErrNotFound
	ErrInvalidObjectId = errors.New("invalid objectId")
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@kesonan kesonan Awaiting requested review from kesonan

At least 1 approving review is required to merge this pull request.

Labels

do-not-merge/hold Indicates that a PR should not merge because of known issues or need more discussion.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /