Code review - Issue 38940043: code review 38940043: gosshnew/ssh/agent: move ssh-agent support into separat...https://codereview.appspot.com/2013-12-10T09:57:12+00:00rietveld
Message from unknown
2013-12-08T00:20:06+00:00hanwen-googleurn:md5:30164fcfcc25c00fe2a95f1abb252e30
Message from unknown
2013-12-08T00:27:51+00:00hanwen-googleurn:md5:69b5fe96e785b17668c8ceb23816845d
Message from unknown
2013-12-08T00:39:24+00:00hanwen-googleurn:md5:dc0db62c8829b7171f9ae6dacf80acc9
Message from unknown
2013-12-08T00:41:37+00:00hanwen-googleurn:md5:0f960f6919535abd7209c830f9b19edf
Message from hanwen@google.com
2013-12-08T00:41:42+00:00hanwen-googleurn:md5:0b59170766a92a3cbd3e6d0da26e4273
Hello agl1, dave@cheney.net, jpsugar@google.com (cc: agl@golang.org, dave@cheney.net, golang-dev@googlegroups.com, hanwen@google.com, jpsugar@google.com),
I'd like you to review this change to
https://hanwen%40google.com@code.google.com/p/gosshnew/
Message from agl@golang.org
2013-12-09T21:12:06+00:00agl1urn:md5:50e6ea59b885ee07433bdd3a4752b1a1
Note that this change doesn't touch agent.go nor agent_test.go. I would have expected them to be deleted.
https://codereview.appspot.com/38940043/diff/60001/ssh/agent/agent.go
File ssh/agent/agent.go (right):
https://codereview.appspot.com/38940043/diff/60001/ssh/agent/agent.go#newcode6
ssh/agent/agent.go:6: Package agent implements a client to a ssh-agent daemon.
s/a/an/
https://codereview.appspot.com/38940043/diff/60001/ssh/agent/agent.go#newcode192
ssh/agent/agent.go:192: // List returns the identities known to the agent.
The comment suggests that the function was going to be renamed, but it hasn't been.
https://codereview.appspot.com/38940043/diff/60001/ssh/agent/agent.go#newcode331
ssh/agent/agent.go:331: sig := Sig{}
var sig Sig
Message from agl@golang.org
2013-12-09T21:12:16+00:00agl1urn:md5:bf5dd03db44efde9d7a47730974a0fa0
LGTM with nits mentioned.
Message from jpsugar@google.com
2013-12-09T21:36:43+00:00jpsugarurn:md5:28ae73921c7e9a8a34ed5c67030a2444
https://codereview.appspot.com/38940043/diff/60001/ssh/agent/agent.go
File ssh/agent/agent.go (right):
https://codereview.appspot.com/38940043/diff/60001/ssh/agent/agent.go#newcode23
ssh/agent/agent.go:23: "code.google.com/p/gosshnew.agent/ssh"
Why is this .agent?
https://codereview.appspot.com/38940043/diff/60001/ssh/agent/agent.go#newcode331
ssh/agent/agent.go:331: sig := Sig{}
On 2013/12/09 21:12:06, agl1 wrote:
> var sig Sig
Or remove the local type:
var sig struct { ... }
Message from hanwen@google.com
2013-12-10T09:56:09+00:00hanwen-googleurn:md5:0b920451cb11907675f54caef1324034
I assume you don't see a delete because Hg has file identity (in contrast to Git), and I ran "hg mv"
https://codereview.appspot.com/38940043/diff/60001/ssh/agent/agent.go
File ssh/agent/agent.go (right):
https://codereview.appspot.com/38940043/diff/60001/ssh/agent/agent.go#newcode6
ssh/agent/agent.go:6: Package agent implements a client to a ssh-agent daemon.
On 2013/12/09 21:12:06, agl1 wrote:
> s/a/an/
Done.
https://codereview.appspot.com/38940043/diff/60001/ssh/agent/agent.go#newcode23
ssh/agent/agent.go:23: "code.google.com/p/gosshnew.agent/ssh"
On 2013/12/09 21:36:43, jpsugar wrote:
> Why is this .agent?
oops! side product of my juggling around 5 CLs in parallel.
https://codereview.appspot.com/38940043/diff/60001/ssh/agent/agent.go#newcode192
ssh/agent/agent.go:192: // List returns the identities known to the agent.
On 2013/12/09 21:12:06, agl1 wrote:
> The comment suggests that the function was going to be renamed, but it hasn't
> been.
Done.
https://codereview.appspot.com/38940043/diff/60001/ssh/agent/agent.go#newcode331
ssh/agent/agent.go:331: sig := Sig{}
On 2013/12/09 21:36:43, jpsugar wrote:
> On 2013/12/09 21:12:06, agl1 wrote:
> > var sig Sig
>
> Or remove the local type:
>
> var sig struct { ... }
Done.
Message from unknown
2013-12-10T09:56:53+00:00hanwen-googleurn:md5:bd39f6f0122932591790297d754ce068
Message from hanwen@google.com
2013-12-10T09:57:12+00:00hanwen-googleurn:md5:8c5653745422f175da4ff6069394ddc9
*** Submitted as https://code.google.com/p/gosshnew/source/detail?r=f904ff1d1391 ***
gosshnew/ssh/agent: move ssh-agent support into separate package.
R=agl, dave, jpsugar
CC=agl, dave, golang-dev, hanwen, jpsugar
https://codereview.appspot.com/38940043