code review 179050043: Clarify Dial docstrings.

Clarify Dial docstrings. Reference net.Dial’s addr scheme where necessary. Improve net/smtp/auth.go “wrong host name” error. Fixes issue 9140.

[Profpatsch, 2014-11-20 17:29:43]

Hello golang-codereviews@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go/

[bradfitz, 2014-11-20 18:26:01]

The tree is frozen right now. Please come back to this after Go 1.4 is out. Also, these need to be in separate changes and need to have the proper subjects, starting with the package names they change. On Thu, Nov 20, 2014 at 9:29 AM, <Ephrones@gmail.com> wrote: > Reviewers: golang-codereviews, > > Message: > Hello golang-codereviews@googlegroups.com, > > I'd like you to review this change to > https://code.google.com/p/go/ > > > Description: > Clarify Dial docstrings. > > Reference net.Dial’s addr scheme where necessary. > Improve net/smtp/auth.go “wrong host name” error. > Fixes issue 9140. > > Please review this at https://codereview.appspot.com/179050043/ > > Affected files (+9, -4 lines): > M src/net/rpc/client.go > M src/net/rpc/jsonrpc/client.go > M src/net/smtp/auth.go > M src/net/smtp/smtp.go > M src/net/smtp/smtp_test.go > > > Index: src/net/rpc/client.go > =================================================================== > --- a/src/net/rpc/client.go > +++ b/src/net/rpc/client.go > @@ -265,7 +265,8 @@ > } > } > > -// Dial connects to an RPC server at the specified network address. > +// Dial connects to an RPC server at the specified network address using > +// net.Dial. > func Dial(network, address string) (*Client, error) { > conn, err := net.Dial(network, address) > if err != nil { > Index: src/net/rpc/jsonrpc/client.go > =================================================================== > --- a/src/net/rpc/jsonrpc/client.go > +++ b/src/net/rpc/jsonrpc/client.go > @@ -114,6 +114,8 @@ > } > > // Dial connects to a JSON-RPC server at the specified network address. > +// The address must include a port number, its formatting is further > described > +// in net.Dial. > func Dial(network, address string) (*rpc.Client, error) { > conn, err := net.Dial(network, address) > if err != nil { > Index: src/net/smtp/auth.go > =================================================================== > --- a/src/net/smtp/auth.go > +++ b/src/net/smtp/auth.go > @@ -66,7 +66,8 @@ > } > } > if server.Name != a.host { > - return "", nil, errors.New("wrong host name") > + return "", nil, fmt.Errorf("wrong host name\n(%s, should > be %s)", > + server.Name, a.host) > } > resp := []byte(a.identity + "\x00" + a.username + "\x00" + > a.password) > return "PLAIN", resp, nil > Index: src/net/smtp/smtp.go > =================================================================== > --- a/src/net/smtp/smtp.go > +++ b/src/net/smtp/smtp.go > @@ -41,7 +41,8 @@ > } > > // Dial returns a new Client connected to an SMTP server at addr. > -// The addr must include a port number. > +// The addr must include a port number, its formatting is further > described in > +// net.Dial. > func Dial(addr string) (*Client, error) { > conn, err := net.Dial("tcp", addr) > if err != nil { > Index: src/net/smtp/smtp_test.go > =================================================================== > --- a/src/net/smtp/smtp_test.go > +++ b/src/net/smtp/smtp_test.go > @@ -79,7 +79,7 @@ > }, > { > server: &ServerInfo{Name: "attacker", TLS: true}, > - err: "wrong host name", > + err: "wrong host name\t(attacker, should be > servername)", > }, > } > for i, tt := range tests { > > > -- > You received this message because you are subscribed to the Google Groups > "golang-codereviews" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to golang-codereviews+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. > 

[ephrones_gmail.com, 2014-11-20 22:38:36]

> Also, these need to be in separate changes and need to have the proper subjects, starting with the package names they change. You mean one for the error message and one for the docstrings? I wonder how to do that … On Thu, Nov 20, 2014 at 7:26 PM, Brad Fitzpatrick <bradfitz@golang.org> wrote: > The tree is frozen right now. Please come back to this after Go 1.4 is out. > > Also, these need to be in separate changes and need to have the proper > subjects, starting with the package names they change. > > > > On Thu, Nov 20, 2014 at 9:29 AM, <Ephrones@gmail.com> wrote: > >> Reviewers: golang-codereviews, >> >> Message: >> Hello golang-codereviews@googlegroups.com, >> >> I'd like you to review this change to >> https://code.google.com/p/go/ >> >> >> Description: >> Clarify Dial docstrings. >> >> Reference net.Dial’s addr scheme where necessary. >> Improve net/smtp/auth.go “wrong host name” error. >> Fixes issue 9140. >> >> Please review this at https://codereview.appspot.com/179050043/ >> >> Affected files (+9, -4 lines): >> M src/net/rpc/client.go >> M src/net/rpc/jsonrpc/client.go >> M src/net/smtp/auth.go >> M src/net/smtp/smtp.go >> M src/net/smtp/smtp_test.go >> >> >> Index: src/net/rpc/client.go >> =================================================================== >> --- a/src/net/rpc/client.go >> +++ b/src/net/rpc/client.go >> @@ -265,7 +265,8 @@ >> } >> } >> >> -// Dial connects to an RPC server at the specified network address. >> +// Dial connects to an RPC server at the specified network address using >> +// net.Dial. >> func Dial(network, address string) (*Client, error) { >> conn, err := net.Dial(network, address) >> if err != nil { >> Index: src/net/rpc/jsonrpc/client.go >> =================================================================== >> --- a/src/net/rpc/jsonrpc/client.go >> +++ b/src/net/rpc/jsonrpc/client.go >> @@ -114,6 +114,8 @@ >> } >> >> // Dial connects to a JSON-RPC server at the specified network address. >> +// The address must include a port number, its formatting is further >> described >> +// in net.Dial. >> func Dial(network, address string) (*rpc.Client, error) { >> conn, err := net.Dial(network, address) >> if err != nil { >> Index: src/net/smtp/auth.go >> =================================================================== >> --- a/src/net/smtp/auth.go >> +++ b/src/net/smtp/auth.go >> @@ -66,7 +66,8 @@ >> } >> } >> if server.Name != a.host { >> - return "", nil, errors.New("wrong host name") >> + return "", nil, fmt.Errorf("wrong host name\n(%s, should >> be %s)", >> + server.Name, a.host) >> } >> resp := []byte(a.identity + "\x00" + a.username + "\x00" + >> a.password) >> return "PLAIN", resp, nil >> Index: src/net/smtp/smtp.go >> =================================================================== >> --- a/src/net/smtp/smtp.go >> +++ b/src/net/smtp/smtp.go >> @@ -41,7 +41,8 @@ >> } >> >> // Dial returns a new Client connected to an SMTP server at addr. >> -// The addr must include a port number. >> +// The addr must include a port number, its formatting is further >> described in >> +// net.Dial. >> func Dial(addr string) (*Client, error) { >> conn, err := net.Dial("tcp", addr) >> if err != nil { >> Index: src/net/smtp/smtp_test.go >> =================================================================== >> --- a/src/net/smtp/smtp_test.go >> +++ b/src/net/smtp/smtp_test.go >> @@ -79,7 +79,7 @@ >> }, >> { >> server: &ServerInfo{Name: "attacker", TLS: true}, >> - err: "wrong host name", >> + err: "wrong host name\t(attacker, should be >> servername)", >> }, >> } >> for i, tt := range tests { >> >> >> >> -- >> You received this message because you are subscribed to the Google Groups >> "golang-codereviews" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to golang-codereviews+unsubscribe@googlegroups.com. >> For more options, visit https://groups.google.com/d/optout. >> > > 

[bradfitz, 2014-11-20 23:42:12]

No, one for rpc and one for smtp On Nov 20, 2014 2:39 PM, "Ephrones Sharp" <ephrones@gmail.com> wrote: > > Also, these need to be in separate changes and need to have the proper > subjects, starting with the package names they change. > > You mean one for the error message and one for the docstrings? > > I wonder how to do that … > > On Thu, Nov 20, 2014 at 7:26 PM, Brad Fitzpatrick <bradfitz@golang.org> > wrote: > >> The tree is frozen right now. Please come back to this after Go 1.4 is >> out. >> >> Also, these need to be in separate changes and need to have the proper >> subjects, starting with the package names they change. >> >> >> >> On Thu, Nov 20, 2014 at 9:29 AM, <Ephrones@gmail.com> wrote: >> >>> Reviewers: golang-codereviews, >>> >>> Message: >>> Hello golang-codereviews@googlegroups.com, >>> >>> I'd like you to review this change to >>> https://code.google.com/p/go/ >>> >>> >>> Description: >>> Clarify Dial docstrings. >>> >>> Reference net.Dial’s addr scheme where necessary. >>> Improve net/smtp/auth.go “wrong host name” error. >>> Fixes issue 9140. >>> >>> Please review this at https://codereview.appspot.com/179050043/ >>> >>> Affected files (+9, -4 lines): >>> M src/net/rpc/client.go >>> M src/net/rpc/jsonrpc/client.go >>> M src/net/smtp/auth.go >>> M src/net/smtp/smtp.go >>> M src/net/smtp/smtp_test.go >>> >>> >>> Index: src/net/rpc/client.go >>> =================================================================== >>> --- a/src/net/rpc/client.go >>> +++ b/src/net/rpc/client.go >>> @@ -265,7 +265,8 @@ >>> } >>> } >>> >>> -// Dial connects to an RPC server at the specified network address. >>> +// Dial connects to an RPC server at the specified network address using >>> +// net.Dial. >>> func Dial(network, address string) (*Client, error) { >>> conn, err := net.Dial(network, address) >>> if err != nil { >>> Index: src/net/rpc/jsonrpc/client.go >>> =================================================================== >>> --- a/src/net/rpc/jsonrpc/client.go >>> +++ b/src/net/rpc/jsonrpc/client.go >>> @@ -114,6 +114,8 @@ >>> } >>> >>> // Dial connects to a JSON-RPC server at the specified network address. >>> +// The address must include a port number, its formatting is further >>> described >>> +// in net.Dial. >>> func Dial(network, address string) (*rpc.Client, error) { >>> conn, err := net.Dial(network, address) >>> if err != nil { >>> Index: src/net/smtp/auth.go >>> =================================================================== >>> --- a/src/net/smtp/auth.go >>> +++ b/src/net/smtp/auth.go >>> @@ -66,7 +66,8 @@ >>> } >>> } >>> if server.Name != a.host { >>> - return "", nil, errors.New("wrong host name") >>> + return "", nil, fmt.Errorf("wrong host name\n(%s, should >>> be %s)", >>> + server.Name, a.host) >>> } >>> resp := []byte(a.identity + "\x00" + a.username + "\x00" + >>> a.password) >>> return "PLAIN", resp, nil >>> Index: src/net/smtp/smtp.go >>> =================================================================== >>> --- a/src/net/smtp/smtp.go >>> +++ b/src/net/smtp/smtp.go >>> @@ -41,7 +41,8 @@ >>> } >>> >>> // Dial returns a new Client connected to an SMTP server at addr. >>> -// The addr must include a port number. >>> +// The addr must include a port number, its formatting is further >>> described in >>> +// net.Dial. >>> func Dial(addr string) (*Client, error) { >>> conn, err := net.Dial("tcp", addr) >>> if err != nil { >>> Index: src/net/smtp/smtp_test.go >>> =================================================================== >>> --- a/src/net/smtp/smtp_test.go >>> +++ b/src/net/smtp/smtp_test.go >>> @@ -79,7 +79,7 @@ >>> }, >>> { >>> server: &ServerInfo{Name: "attacker", TLS: true}, >>> - err: "wrong host name", >>> + err: "wrong host name\t(attacker, should be >>> servername)", >>> }, >>> } >>> for i, tt := range tests { >>> >>> >>> >>> -- >>> You received this message because you are subscribed to the Google >>> Groups "golang-codereviews" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to golang-codereviews+unsubscribe@googlegroups.com. >>> For more options, visit https://groups.google.com/d/optout. >>> >> >> > -- > You received this message because you are subscribed to the Google Groups > "golang-codereviews" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to golang-codereviews+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. > 

[ephrones_gmail.com, 2014-11-21 15:43:04]

> No, one for rpc and one for smtp imho those changes belong together, but as you wish. 

[bradfitz, 2014-11-21 18:39:26]

Also no newlines in error messages. On Fri, Nov 21, 2014 at 7:43 AM, Ephrones Sharp <ephrones@gmail.com> wrote: > > No, one for rpc and one for smtp > > imho those changes belong together, but as you wish. > 

[gobot, 2014-12-19 05:16:40]

R=close To the author of this CL: The Go project has moved to Gerrit Code Review. If this CL should be continued, please see the latest version of https://golang.org/doc/contribute.html for instructions on how to set up Git and the Go project's Gerrit codereview plugin, and then create a new change with your current code. If there has been discussion on this CL, please give a link to it (golang.org/cl/179050043 is best) in the description in your new CL. Thanks very much.