Skip to content

Return a struct from gin's RegisterHandlers instead of an interface - #555

Closed
jazware wants to merge 2 commits into
oapi-codegen:mainfrom
jazware:gin_register_return_interface
Closed

Return a struct from gin's RegisterHandlers instead of an interface#555
jazware wants to merge 2 commits into
oapi-codegen:mainfrom
jazware:gin_register_return_interface

Conversation

@jazware

@jazware jazware commented Apr 19, 2022

Copy link
Copy Markdown
Contributor

This change provides an alternative to #554 based on the discussions in #530.

For setting up a Gin engine, we want to handle the IRouter interface when registering handlers, but as this only thinly wraps a *gin.GroupRouter, we can return a *gin.GroupRouter from the registration function.

We could refactor the RegisterHandlers and RegisterHandlersWithOptions to just take a *gin.GroupRouter to begin with, I'm not really sure what we gain by using the IRouter interface here. Does anyone have opinions on doing that?

@jazware

jazware commented Apr 19, 2022

Copy link
Copy Markdown
Contributor Author

I included autogeneration of new examples via go generate directives in the gin example folder that were missing which is likely what lead to this breaking change being merged without notice. If the changes make the generated code change in a breaking way, it will fail in make test now when it tries to build the example packages. make test runs after make generate so we should catch breaking changes this way.

@k0mmsussert0d

Copy link
Copy Markdown

Hi! Sorry to nudge you like this, but could you provide any estimate on when this is going to be merged? #485 poses major discrepancy when using this generator. I can see a solution had already been implemented in the past, but got reverted later due to inconsistencies with usage examples. If that's the only issue, would it be much of a trouble for you to align the documentation to match the actual API?

Afaik this project had some breaking changes in the release a week or two ago, perhaps now it's a good time to carry this change out.

@mromaszewicz

Copy link
Copy Markdown
Member

Thank you for submitting this PR, @jazware — but we need to close it. Goal (1) — accepting gin.IRouter instead of *gin.Engine — was fully achieved by #879 (commit e8dc342d). Goal (2) — returning a concrete *gin.RouterGroup instead of *gin.Engine — was resolved differently: the maintainers ultimately chose to make RegisterHandlers void, a different design decision. So while the input-type concern is fixed, the return-type direction wasn't taken.

Apologies for the long wait, and thanks for the contribution.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants