FAQ
Reviewers: agl1, dfc,

Message:
Hello agl@golang.org, dave@cheney.net (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go.crypto


Description:
go.crypto/ssh: Add support for ECDSA keys and certs.

Please review this at https://codereview.appspot.com/6873060/

Affected files:
M ssh/certs.go
M ssh/common.go
M ssh/keys.go

Search Discussions

  • Jmpittman at Dec 11, 2012 at 1:22 am
    Hello agl@golang.org, dave@cheney.net (cc: golang-dev@googlegroups.com),

    Please take another look.


    https://codereview.appspot.com/6873060/
  • Jmpittman at Dec 11, 2012 at 2:33 pm
    Hello agl@golang.org, dave@cheney.net (cc: golang-dev@googlegroups.com),

    Please take another look.


    https://codereview.appspot.com/6873060/
  • Jmpittman at Dec 11, 2012 at 3:57 pm
    Hello agl@golang.org, dave@cheney.net (cc: golang-dev@googlegroups.com),

    Please take another look.


    https://codereview.appspot.com/6873060/
  • Agl at Dec 11, 2012 at 3:57 pm
    LGTM. I'll wait a bit to see whether dfc has anything to note.

    If I forget about it in 48 hours, please ping :)


    https://codereview.appspot.com/6873060/diff/3004/ssh/certs.go
    File ssh/certs.go (right):

    https://codereview.appspot.com/6873060/diff/3004/ssh/certs.go#newcode8
    ssh/certs.go:8: package ssh
    I don't think the References comment should live above the package line.

    https://codereview.appspot.com/6873060/
  • Jmpittman at Dec 11, 2012 at 4:00 pm
    Thanks for the quick response. I think I covered most everything with
    the last two updates to the CL. I left the Authorized Keys stuff alone
    (for now) for the parsing (where it clearly indicates not supporting
    beyond ssh-rsa and ssh-dss). I think that could be saved for a later
    CL.


    https://codereview.appspot.com/6873060/diff/3004/ssh/certs.go
    File ssh/certs.go (right):

    https://codereview.appspot.com/6873060/diff/3004/ssh/certs.go#newcode8
    ssh/certs.go:8: package ssh
    On 2012/12/11 15:32:16, agl1 wrote:
    I don't think the References comment should live above the package
    line.

    Unless you feel it not necessary, I moved it up so that it would show up
    in package documentation via godoc. Godoc seems to create a nice little
    box for references, but only when they appear above the package line.
    An example is http://golang.org/pkg/archive/tar/.

    Although, after double checking, it is not showing up when I run a local
    copy of godoc. It may be that I would have to put the reference comment
    in another file. I will move it back for now.

    https://codereview.appspot.com/6873060/
  • Dave Cheney at Dec 11, 2012 at 9:52 pm
    Sorry I haven't contributed much in the way of reviewing ssh patches,
    this is very poor form on my part, I'll have some time after work
    today to review.
    On Wed, Dec 12, 2012 at 2:52 AM, wrote:
    Thanks for the quick response. I think I covered most everything with
    the last two updates to the CL. I left the Authorized Keys stuff alone
    (for now) for the parsing (where it clearly indicates not supporting
    beyond ssh-rsa and ssh-dss). I think that could be saved for a later
    CL.



    https://codereview.appspot.com/6873060/diff/3004/ssh/certs.go
    File ssh/certs.go (right):

    https://codereview.appspot.com/6873060/diff/3004/ssh/certs.go#newcode8
    ssh/certs.go:8: package ssh
    On 2012/12/11 15:32:16, agl1 wrote:

    I don't think the References comment should live above the package
    line.

    Unless you feel it not necessary, I moved it up so that it would show up
    in package documentation via godoc. Godoc seems to create a nice little
    box for references, but only when they appear above the package line.
    An example is http://golang.org/pkg/archive/tar/.

    Although, after double checking, it is not showing up when I run a local
    copy of godoc. It may be that I would have to put the reference comment
    in another file. I will move it back for now.

    https://codereview.appspot.com/6873060/

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedDec 11, '12 at 1:12a
activeDec 11, '12 at 9:52p
posts7
users3
websitegolang.org

3 users in discussion

Jmpittman: 5 posts Dave Cheney: 1 post Agl: 1 post

People

Translate

site design / logo © 2022 Grokbase