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
Refactor pktline #1082
base: v6-exp
Are you sure you want to change the base?
Refactor pktline #1082
Conversation
Git file, git, and ssh transports use a full-duplex transport protocol. This is now located under `plumbing/transport`. The server transport along with its `Serve*` methods are now located under `plumbing/server`.
Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com>
Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com>
Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com>
Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com>
Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com>
Pktline is already implied in the package name, no need to have "Packet" in the method names.
Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com>
1ec5a4c
to
33cd06f
Compare
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.
Small nits, otherwise LGTM.
|
||
// Make sure you close the server after the test. | ||
func SetupProxyServer(c *C, handler http.Handler, isTls, schemaAddr bool) (string, *http.Server, net.Listener) { | ||
httpListener, err := net.Listen("tcp", ":0") |
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 is to wide, we want to restrict the listening to the loopback interface.
httpListener, err := net.Listen("tcp", ":0") | |
httpListener, err := net.Listen("tcp", "127.0.0.1:0") |
} | ||
|
||
return true | ||
s.n, s.err = Read(s.r, s.buf[:]) |
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.
Please add a nil check for s.r
.
return d.Decode(req) | ||
} | ||
|
||
type updReqDecoder struct { | ||
r io.ReadCloser | ||
s *pktline.Scanner | ||
s io.Reader |
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.
The field s
sounds confusing now that this is no longer a scanner.
s io.Reader | |
pr io.Reader |
mtx sync.Mutex | ||
) | ||
|
||
// Register adds or modifies an existing protocol. |
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.
Where the API changed substantially, I think we should either add comments to clarify that, or find another place to amass all the API changes, so it is easier for users to migrate to V6.
// Register adds or modifies an existing protocol. | |
// Register adds or modifies an existing protocol. | |
// Equivalent to client.InstallProtocol in go-git before V6. |
defer mtx.Unlock() | ||
registry[protocol] = c |
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.
When the code is this simple it is not worth the additional costs of a defer
.
defer mtx.Unlock() | |
registry[protocol] = c | |
registry[protocol] = c | |
mtx.Unlock() |
defer mtx.Unlock() | ||
delete(registry, scheme) |
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.
defer mtx.Unlock() | |
delete(registry, scheme) | |
delete(registry, scheme) | |
defer mtx.Unlock() |
Taken from aymanbagabas#1