| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
go.mod: update to github.com/emicklei/go-restful/v3 v3.7.3 #6337
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
Conversation
|
Build succeeded.
|
|
I should add that it looks to be non-trivial to update this dependency in kubernetes (see kubernetes/kubernetes#106841 and kubernetes/kube-openapi#271), so we won't be able to get rid of the old version (yet). Updating the dependency should not be "critical" (assuming the old version is still somewhat maintained). I guess we could either;
|
|
Personally I prefer "one step into upgrading" approach. Someone need to take the first step. |
Agreed. I don't think this PR changes any of the public signatures, so should probably be fine (but I appreciate the extra eyes to be sure I didn't overlook any) |
|
@AkihiroSuda @dims ptal |
|
@fedebongio this PR primarily affects api-machinery from what i can tell, can you please assign someone to review this? /assign @fedebongio |
|
@dims: GitHub didn't allow me to assign the following users: fedebongio. Note that only containerd members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@fedebongio @dims can you take a look? I thought I could assign to him manually, but not. |
|
@kzys apologies, i assumed this was the k/k repository somehow |
|
@dmcgowan please take a look to see if we need to do this before 1.6? |
|
Synced-up with @dims. According to him;
So I'm adding this one to the 1.7 milestone. |
|
This PR has merge conflicts now, just FYI @thaJeztah |
full diff: emicklei/go-restful@v2.9.5...v3.7.3 - Switch to using go modules - Add check for wildcard to fix CORS filter - Add check on writer to prevent compression of response twice - Add OPTIONS shortcut WebService receiver - Add Route metadata to request attributes or allow adding attributes to routes - Add wroteHeader set - Enable content encoding on Handle and ServeHTTP - Feat: support google custom verb - Feature: override list of method allowed without content-type - Fix Allow header not set on '405: Method Not Allowed' responses - Fix Go 1.15: conversion from int to string yields a string of one rune - Fix WriteError return value - Fix: use request/response resulting from filter chain - handle path params with prefixes and suffixes - HTTP response body was broken, if struct to be converted to JSON has boolean value - List available representations in 406 body - Support describing response headers - Unwrap function in filter chain + remove unused dispatchWithFilters Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
force-pushed
the
bump_go_restful
branch
from
e274930 to
481fb92
Compare
|
Thanks for the ping; rebased |
|
Build succeeded.
|
|
@eStep Can you take a look and merge this change? Seems good to go. |
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.
LGTM
still some indirect dependencies using the non-go modules version (see kubernetes/kubernetes#106841, kubernetes/kube-openapi#271)
full diff: emicklei/go-restful@v2.9.5...v3.7.3
Signed-off-by: Sebastiaan van Stijn github@gone.nl