Skip to content
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

Proposal: Improve hz template documentation #195

Closed
Haswf opened this issue Aug 14, 2022 · 1 comment
Closed

Proposal: Improve hz template documentation #195

Haswf opened this issue Aug 14, 2022 · 1 comment

Comments

@Haswf
Copy link
Contributor

Haswf commented Aug 14, 2022

Is your feature request related to a problem? Please describe.

I'am trying to define a handler struct for each service defined idl, which is a widely used pattern in API development.
Taking hertz-examples/tree/main/hz/template/package as an example, what the package template offers is:

// HelloMethod .
// @router /hello [GET]
func HelloMethod(ctx context.Context, c *app.RequestContext) {
	//  you can code something
	var err error
	var req example.HelloReq
	err = c.BindAndValidate(&req)
	if err != nil {
		c.String(400, err.Error())
		return
	}

	resp := new(example.HelloResp)

	c.JSON(200, resp)
}

What I am looking for is the follwoing:

type HelloHandler struct {
	
}
// HelloMethod .
// @router /hello [GET]
func (h *HelloHandler) HelloMethod(ctx context.Context, c *app.RequestContext) {
	//  you can code something
	var err error
	var req example.HelloReq
	err = c.BindAndValidate(&req)
	if err != nil {
		c.String(400, err.Error())
		return
	}

	resp := new(example.HelloResp)

	c.JSON(200, resp)
}

I found it challenging to follow the documentation becuase of the following reasons:

  1. The default package.yaml file is not available to download as a file. The full template is way too long to copy, plus there is no copy button.
  2. The lack of documentaion on which file is customizable. After reading documentation, It turned out handler.go ,router.go, register.go, middleware.go, client.go, handler_single.go, middleware_single.go are paths that can be modified. This is not really reader friendly as we have to go thourgh the whole file to understnad what's happening here.
  3. There is no explaining on what variables are avaialble in each template. For example, handler.go explicitly refers to .PackageName, .Imports, .Methods, .Serializer but there is no clarifaction on what variable we are able to refer to when exectuing this template. It took me a few trial and error to figure out .ServiceName is not accessible in the handler.go template.
  - path: handler.go
    delims:
      - '{{'
      - '}}'
    body: |-
      // this is my custom handler.

      package {{.PackageName}}

      import (
      	"context"

      	"github.com/cloudwego/hertz/pkg/app"

      {{- range $k, $v := .Imports}}
      	{{$k}} "{{$v.Package}}"
      {{- end}}
      )

      {{range $_, $MethodInfo := .Methods}}
      {{$MethodInfo.Comment}}
      func {{$MethodInfo.Name}}(ctx context.Context, c *app.RequestContext) {
        //  you can code something
      	var err error
      	{{if ne $MethodInfo.RequestTypeName "" -}}
      	var req {{$MethodInfo.RequestTypeName}}
      	err = c.BindAndValidate(&req)
      	if err != nil {
      		c.String(400, err.Error())
      		return
      	}
      	{{end}}
      	resp := new({{$MethodInfo.ReturnTypeName}})

      	c.{{.Serializer}}(200, resp)
      }
      {{end}}

Describe the solution you'd like

We can use the following layout. It helps to nevigate to the exactly section which user want to modify, say I only want to modify handler, why should I go through and understand the whole file?

  1. Customize Package Template
    1.1 how to get default template
    1.2 how to specify template file in command line
    1.3 breakdown of each section
    1.3.1 handler.go (includes breif explaination, default value, a list of available variables and their type (i.e. .VAR). I guess go doc would really helps in this case)
    1.3.2 ...

The same structure could be applied to customised layout as well.

Describe alternatives you've considered
I believe it's a better idea to bundle template files with hz release. One possible way is providing additional flags to generate /template/package.yaml or/and /template/layout.yaml when creating a new project.

Additional context

None.

@FGYFFFF
Copy link
Contributor

FGYFFFF commented Aug 15, 2022

@Haswf
Thanks for your advice!
I will answer you based on your three questions.

  • All templates are available at hertz-example and the documentation I should not mark. And there is really no copy button on the document.
  • In theory, all files can be customized. But at the moment it is not recommended to modify the "register.go" file due to update command. However, I understand that the average user only need to modify handler.go and middleware.go.
  • Previously I thought the variable names in the template would explain what each variable does, but now it seems a bit cumbersome. I will provide the explanation of the variable names in the template. Also, why do you use the ".ServiceName" variable in "handler.go"?

I think the documentation layout you gave is very good, would you like to propose a "PR" based on your idea? I will also work with you on this

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

No branches or pull requests

2 participants