-
Notifications
You must be signed in to change notification settings - Fork 8
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
Implement interface for gin #121
Conversation
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.
Hi @VishvaNavanjana! Thank you for the PR, I think that add gin-gonic to the repo would be awesome!
Can you add unit and integration tests? You can look at those made for other supported routers.
switch method { | ||
case http.MethodGet: | ||
return r.router.GET(path, gin.HandlerFunc(handler)) | ||
case http.MethodPost: | ||
return r.router.POST(path, gin.HandlerFunc(handler)) | ||
case http.MethodPut: | ||
return r.router.PUT(path, gin.HandlerFunc(handler)) | ||
case http.MethodDelete: | ||
return r.router.DELETE(path, gin.HandlerFunc(handler)) | ||
default: | ||
return r.router.Any(path, gin.HandlerFunc(handler)) | ||
} |
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.
This switch/case can lead to unexpected behaviour (for example, if set PATCH method this will set Any methods, which are GET, POST, PUT, PATCH, HEAD, OPTIONS, DELETE, CONNECT, TRACE).
What do you think about replace it with the match method?
Something like:
switch method { | |
case http.MethodGet: | |
return r.router.GET(path, gin.HandlerFunc(handler)) | |
case http.MethodPost: | |
return r.router.POST(path, gin.HandlerFunc(handler)) | |
case http.MethodPut: | |
return r.router.PUT(path, gin.HandlerFunc(handler)) | |
case http.MethodDelete: | |
return r.router.DELETE(path, gin.HandlerFunc(handler)) | |
default: | |
return r.router.Any(path, gin.HandlerFunc(handler)) | |
} | |
return r.router.Match([]string{method}, path, handler) |
} | ||
|
||
func (r ginRouter) TransformPathToOasPath(path string) string { | ||
return path |
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.
Gin seems to support path params with colon, is it true? It should be converted to oas spec with this. Can you see an example usage in fiber support.
No description provided.