Skip to content

Conversation

jaronnie
Copy link
Contributor

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
Preview

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 {
return err
}
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
Preview

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

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
@jaronnie
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.

@jaronnie
Copy link
Contributor Author

jaronnie commented Sep 6, 2025

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
Labels
do-not-merge/hold Indicates that a PR should not merge because of known issues or need more discussion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants