-
Notifications
You must be signed in to change notification settings - Fork 164
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
Support mod feature #43
base: master
Are you sure you want to change the base?
Changes from 5 commits
bc7df92
d333677
1afd978
81c4418
29395be
1e719fa
f22a832
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,3 +18,4 @@ vendor/ | |
|
||
.idea | ||
*.iml | ||
.vscode/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1683,35 +1683,39 @@ func (g *generateCmd) generateRun() (*PartialGenerator, error) { | |
jen.Lit("URL"), | ||
jen.Id("*zipkinURL"), | ||
), | ||
jen.List(jen.Id("collector"), jen.Err()).Op(":=").Qual( | ||
"github.com/openzipkin/zipkin-go-opentracing", "NewHTTPCollector", | ||
jen.Id("reporter").Op(":=").Qual( | ||
"github.com/openzipkin/zipkin-go/reporter/http", "NewReporter", | ||
).Call(jen.Id("*zipkinURL")), | ||
jen.Defer().Id("reporter").Dot("Close").Call(), | ||
jen.List(jen.Id("endpoint"), jen.Id("err")).Op(":=").Qual( | ||
"github.com/openzipkin/zipkin-go", "NewEndpoint", | ||
).Call( | ||
jen.Lit(g.name), | ||
jen.Lit("localhost:80"), | ||
), | ||
jen.If(jen.Err().Op("!=").Nil()).Block( | ||
jen.Id("logger").Dot("Log").Call( | ||
jen.Lit("err"), | ||
jen.Id("err"), | ||
), | ||
jen.Qual("os", "Exit").Call(jen.Lit(1)), | ||
), | ||
jen.Defer().Id("collector").Dot("Close").Call(), | ||
jen.Id("recorder").Op(":=").Qual( | ||
"github.com/openzipkin/zipkin-go-opentracing", "NewRecorder", | ||
).Call( | ||
jen.Id("collector"), | ||
jen.Lit(false), | ||
jen.Lit("localhost:80"), | ||
jen.Lit(g.name), | ||
), | ||
jen.List(jen.Id("tracer"), jen.Id("err")).Op("=").Qual( | ||
"github.com/openzipkin/zipkin-go-opentracing", "NewTracer", | ||
).Call(jen.Id("recorder")), | ||
jen.Id("localEndpoint").Op(":=").Qual("github.com/openzipkin/zipkin-go", "WithLocalEndpoint").Call(jen.Id("endpoint")), | ||
jen.List(jen.Id("nativeTracer"), jen.Id("err")).Op(":=").Qual( | ||
"github.com/openzipkin/zipkin-go", "NewTracer", | ||
).Call(jen.Id("reporter"), jen.Id("localEndpoint")), | ||
jen.If(jen.Err().Op("!=").Nil()).Block( | ||
jen.Id("logger").Dot("Log").Call( | ||
jen.Lit("err"), | ||
jen.Id("err"), | ||
), | ||
jen.Qual("os", "Exit").Call(jen.Lit(1)), | ||
), | ||
jen.Id("tracer").Op("=").Qual( | ||
"github.com/openzipkin-contrib/zipkin-go-opentracing", "Wrap", | ||
).Call( | ||
jen.Id("nativeTracer"), | ||
), | ||
).Else().If(jen.Id("*lightstepToken").Op("!=").Lit("")).Block( | ||
jen.Id("logger").Dot("Log").Call( | ||
jen.Lit("tracer"), | ||
GrantZheng marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
@@ -1728,8 +1732,8 @@ func (g *generateCmd) generateRun() (*PartialGenerator, error) { | |
), | ||
), | ||
jen.Defer().Qual( | ||
"github.com/lightstep/lightstep-tracer-go", "FlushLightStepTracer", | ||
).Call(jen.Id("tracer")), | ||
"github.com/lightstep/lightstep-tracer-go", "Flush", | ||
).Call(jen.Id("context.Background()"), jen.Id("tracer")), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You need to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi, I fix it and update the pr |
||
).Else().If(jen.Id("*appdashAddr").Op("!=").Lit("")).Block( | ||
jen.Id("logger").Dot("Log").Call( | ||
jen.Lit("tracer"), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ package generator | |
|
||
import ( | ||
"fmt" | ||
"os/exec" | ||
"path" | ||
"strings" | ||
|
||
|
@@ -40,6 +41,12 @@ func NewNewService(name string) Gen { | |
// Generate will run the generator. | ||
func (g *NewService) Generate() error { | ||
g.CreateFolderStructure(g.destPath) | ||
err := g.genModule() | ||
if err != nil { | ||
println(err.Error()) | ||
return err | ||
} | ||
|
||
comments := []string{ | ||
"Add your methods here", | ||
"e.x: Foo(ctx context.Context,s string)(rs string, err error)", | ||
|
@@ -53,3 +60,19 @@ func (g *NewService) Generate() error { | |
) | ||
return g.fs.WriteFile(g.filePath, g.srcFile.GoString(), false) | ||
} | ||
|
||
func (g *NewService) genModule() error { | ||
exist, _ := g.fs.Exists(g.name + "/go.mod") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we want to add the dependencies manually, at least the once we depend on after running There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @kujtimiihoxha :) Why we need to maintain multiple services under the same package? The other scene is that we want to construct a monolithic project that contains multiple services under the same package. There have some dependency between services and the monolithic project is started only by one main.go file which needs to be create manually at the service's parent level. In addition, we should also delete the go.mod file at the service level and create a go.mod file manually at the parent level. About the one scene, I think it is of small significance and rarely used in this way. We only need to cover most of the usage scenarios. There's no need to support. About the other scene, I think it is significant. But I do not recommend realizing the function at the kit tool. The responsibility of kit tool is just construct our services. If we want to construct a monolithic project, we should do manually as follows:
What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you are right. One other thing that came to my mind now is that with how it is implemented now we are not maintaining backwards compatibility with already existing projects that use kit. Is there a way to only use modules when the user wants to do that ? Basically only use mod if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. About compatible with the already existing projects,we may have two ways, I think. The one is that we should use tag vesion to manage our feature, such as checkout two new branches named release/v0.1.0 and release/v0.2.0 from the master and tag the release/v0.1.0 branch code v0.1.0 which should run kit commands under the GOPATH, and then switch to release/v0.2.0 branch, merge code from feature/mod_support branch, tag v0.2.0 under the release/v0.2.0 branch. If the user want to use the GOPATH ,he can use the v0.1.0 version; if the user want to use the module feature, he can use the v0.2.0 version. The other is that continue to support GOPATH in our new version, if user run I think the first way is better than the second. I want to quote some word from https://blog.golang.org/modules2019.
So, I think there is no need to compatible with GOPATH in our new version. We should guide the user to use the new module feature. If the user have to use the GOPATH, he could switch to the old version v0.1.0. What do you think about this? :) |
||
if exist { | ||
return nil | ||
} | ||
|
||
moduleName := g.name | ||
if viper.GetString("n_s_module") != "" { | ||
moduleName = viper.GetString("n_s_module") | ||
} | ||
cmdStr := "cd " + g.name + " && go mod init " + moduleName | ||
cmd := exec.Command("sh", "-c", cmdStr) | ||
_, err := cmd.Output() | ||
return err | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.