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.

Sign up for GitHub

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

Jump to bottom

[Go] Add a generator for the gin framework #1048

Merged
merged 9 commits into from Sep 21, 2018
Merged

[Go] Add a generator for the gin framework #1048

merged 9 commits into from Sep 21, 2018

Conversation

kemokemo
Copy link
Contributor

@kemokemo kemokemo commented Sep 17, 2018 •
edited by wing328

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: master, 3.3.x, 4.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

@antihax @bvwells @grokify

Description of the PR

Issue #1047

I respect the openapi-generator.
I would be pleased if you could give us your guidance.

Ref: Go Gin framework https://github.com/gin-gonic/gin

@antihax
Copy link
Contributor

antihax commented Sep 17, 2018

I would try to condense the java changes into the main go server/abstract generator and only create a separate template for the Gin server; if this is at all possible.

@kemokemo
Copy link
Contributor Author

@antihax Thank you very much for your review and comment!
I am sorry that the majority of Java code has been duplicated...

@wing328
Copy link
Member

wing328 commented Sep 17, 2018 •
edited

IMO it's ok to extends Go Gin generator just with Abstract Go class for day-1 requirement. If we see more Go server generators contributed by the community, we can refactor common code into an Abstract Go server class similar to what we've done for Java JAX-RS generators

wing328
wing328 reviewed Sep 17, 2018
View reviewed changes
...s/openapi-generator/src/main/java/org/openapitools/codegen/languages/GoGinServerCodegen.java Outdated
@@ -0,0 +1,180 @@
/*
* Copyright 2018 OpenAPI-Generator Contributors (https://openapi-generator.tech)
* Copyright 2018 SmartBear Software
Copy link
Member

Choose a reason for hiding this comment

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

@kemokemo I believe we no longer need line 3 for new files added directly to this project (OpenAPI Generator). Please refer to https://opensource.stackexchange.com/a/7301/12997 for more information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your review and the information about the 'Copyright notice in the file header'.
I'll delete these 3 lines.

wing328
wing328 reviewed Sep 17, 2018
View reviewed changes
...s/openapi-generator/src/main/java/org/openapitools/codegen/languages/GoGinServerCodegen.java Outdated
public String getHelp() {
return "Generates a Go server library using OpenAPI-Generator. By default, " +
"it will also generate service classes -- which you can disable with the `-Dnoservice` environment variable.";
}
Copy link
Member

Choose a reason for hiding this comment

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

@kemokemo I think we will need to revise the help text, e.g. to mention about Go Gin framework.

Does this generator also support the -Dnoservice option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh.. Sorry.
I'll make the help message more appropriate and delete the mention about -Dnoservice.

@kemokemo
Copy link
Contributor Author

I had pushed some fixes.
Would you please confirm these changes? Thank you!

@wing328 wing328 added this to the 3.3.0 milestone Sep 20, 2018
@wing328
Copy link
Member

wing328 commented Sep 20, 2018 •
edited

Sorry I wasn't clear. I mean still keeping the following line to give us (including you) for the contribution:

 * Copyright 2018 OpenAPI-Generator Contributors (https://openapi-generator.tech)

I've pushed 896d07b with the following:

  • add back Copyright 2018 OpenAPI-Generator Contributors (https://openapi-generator.tech)
  • rename ./bin/go-gin-petstore.sh to ./bin/go-gin-petstore-server.sh to make it consistent with other server scripts
  • update the samples with gofmt using export GO_FMT_PATH=/usr/local/bin/gofmt
  • add the script to ./bin/utils/ensure-up-to-date to ensure the samples are up-to-date moving forward

If no further question/feedback on this PR, I'll merge it tomorrow (Friday)

cc @antihax @bvwells @grokify

UPDATE: we'll later rename GO_FMT_PATH to GO_POST_PROCESS_FILE in a separate PR to make the env variable naming consistent across all generators.

@kemokemo
Copy link
Contributor Author

kemokemo commented Sep 20, 2018 •
edited

I had neglected confirmation. I'm sorry.
Thank you so much for your support!
I am proud that I had committed to OpenAPI Generator 😃

@wing328
Copy link
Member

wing328 commented Sep 21, 2018

I did some tests and was able to run it directly and via Docker. Partial result below:

[GIN-debug] POST   /v2/user                  --> _/go/src/go.CreateUser (3 handlers)
[GIN-debug] POST   /v2/user/createWithArray  --> _/go/src/go.CreateUsersWithArrayInput (3 handlers)
[GIN-debug] POST   /v2/user/createWithList   --> _/go/src/go.CreateUsersWithListInput (3 handlers)
[GIN-debug] DELETE /v2/user/{username}       --> _/go/src/go.DeleteUser (3 handlers)
[GIN-debug] GET    /v2/user/{username}       --> _/go/src/go.GetUserByName (3 handlers)
[GIN-debug] GET    /v2/user/login            --> _/go/src/go.LoginUser (3 handlers)
[GIN-debug] GET    /v2/user/logout           --> _/go/src/go.LogoutUser (3 handlers)
[GIN-debug] PUT    /v2/user/{username}       --> _/go/src/go.UpdateUser (3 handlers)
[GIN-debug] Listening and serving HTTP on :8080

@wing328 wing328 merged commit 14de289 into OpenAPITools:master Sep 21, 2018
@wing328
Copy link
Member

wing328 commented Sep 21, 2018

PR merged into master. Thank you for your contribution 👍

@wing328
Copy link
Member

wing328 commented Sep 21, 2018

Reddit: https://www.reddit.com/r/golang/comments/9hs0h6/go_gin_server_generator_is_the_96th_generator_to/

Twitter: https://twitter.com/oas_generator/status/1043183765982302208

Hacker News: https://news.ycombinator.com/item?id=18041132

@kemokemo
Copy link
Contributor Author

I am glad that I could contribute to a wonderful project!
Thank you very much! 👍

@wing328
Copy link
Member

wing328 commented Sep 21, 2018

@kemokemo please get some rest as it's late in Japan. I'll talk to you on Monday about further enhancement to the generator. Have a nice weekend.

@kemokemo kemokemo deleted the add-go-gin branch September 22, 2018 23:57
@kemokemo kemokemo restored the add-go-gin branch September 22, 2018 23:57
@kemokemo kemokemo mentioned this pull request Sep 26, 2018
Closed
@wing328
Copy link
Member

wing328 commented Oct 2, 2018

@kemokemo thanks again for the new generator, which is included in the v3.3.0 minor release: https://twitter.com/oas_generator/status/1046941449609068544

@kemokemo
Copy link
Contributor Author

kemokemo commented Oct 2, 2018

@wing328 Thank you so much for your support! I'm very happy 😄

A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
@kemokemo @wing328
[Go] Add a generator for the gin framework (OpenAPITools#1048)
495c963 
* copy from go-server and add go-gin-server generator

* change the templates for the gin

* fix warnings of the golint tool

* fix the path of script

* add samples

* delete unnecessary comments (OpenAPITools#1048)

* make the help message more appropriate (OpenAPITools#1048)

* fix the link address format (OpenAPITools#1048)

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

@wing328 wing328 wing328 left review comments

Assignees
No one assigned
Labels
Enhancement: New generator Server: Go
Projects
None yet
Milestone
3.3.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@kemokemo @antihax @wing328 @ackintosh