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

Add Go echo server codegen #9224

Merged
merged 22 commits into from
Apr 21, 2021
Merged

Conversation

ph4r5h4d
Copy link
Contributor

@ph4r5h4d ph4r5h4d commented Apr 9, 2021

This is the first draft for go-echo-server codegen for OpenAPI-codegen. the purpose is that we are internally using Go Echo, and we needed to create this codegen. Still, I thought that it might be worth sharing it with everyone.
Related issue: #9093

I'm not a Java guru, and I mostly used go-gin-codegen codes to build this new codegen, and it's a bit opinionated.

Kindly let me know what needs to be done so we can merge this PR.

@antihax @grokify @kemokemo @jirikuncar
Kindly let me know what needs to be improved.

@kemokemo
Copy link
Contributor

@ph4r5h4d
Thank you so much for your PR! 😄 It looks nice.
I'm very happy about this PR because I also used echo for my company's internal web service and like it.
I like echo as well as gin.

I have confirmed that the generated go-echo-server's code builds, runs, and works as a container.
It looks good, but there are a few things I would like to discuss. Can you please see the following?

golint ./... reports some issues

I feel that it would be better to add comments for methods exported in the package, so that issues reported by the golint tool can be resolved. What do you think?

docker run

This may be a problem only in my macOS environment. I had to add the -p flag when running the docker container.
How about in your environment?

  • (I can not access to this app) docker run --rm -it openapi
  • (I can access to this app) docker run --rm -p 8080:8080 -it openapi

@ph4r5h4d
Copy link
Contributor Author

I'll take a look into the items you've mentioned and will issue an update by the end of the week.
Thank you for putting time into testing this PR.

@ph4r5h4d
Copy link
Contributor Author

Can you kindly check again and see if everything is fine?

@wing328
Copy link
Member

wing328 commented Apr 21, 2021

Overall it looks good to start with 👍

I'll merge this and file another PR with minor enhancements.

Thanks for the new generator!

@wing328 wing328 merged commit 733a180 into OpenAPITools:master Apr 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants