code review 196044: Regularize the comments for the websocket package and d...

Regularize the comments for the websocket package and document all functions and methods.

[r, 2010-01-28 03:48:40]

Hello rsc, ukai@google.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change.

[rsc1, 2010-01-28 03:53:00]

LGTM

http://codereview.appspot.com/196044/diff/1/3
File src/pkg/websocket/server.go (right):

http://codereview.appspot.com/196044/diff/1/3#newcode22
src/pkg/websocket/server.go:22: )
looks like a one-space indent.
is that intended?

http://codereview.appspot.com/196044/diff/1/4
File src/pkg/websocket/websocket.go (right):

http://codereview.appspot.com/196044/diff/1/4#newcode52
src/pkg/websocket/websocket.go:52: func (ws *Conn) Read(msg []byte) (n int, err
os.Error) {
no comment here?

More generally, I think it's fine to put comments on all these,
but they're just trying to satisfy the net.Conn interface.
Maybe it suffices to say in the type comment that it
implements net.Conn and not bother with the 
per-method comments?

http://codereview.appspot.com/196044/diff/1/4#newcode113
src/pkg/websocket/websocket.go:113: // Write implements the io.Closer interface
for a Conn.
s/Write/Close/

http://codereview.appspot.com/196044/diff/1/4#newcode130
src/pkg/websocket/websocket.go:130: // SetTimeout sets the connection's network
read timeout in nanoseconds.
SetReadTimeout

http://codereview.appspot.com/196044/diff/1/4#newcode138
src/pkg/websocket/websocket.go:138: // SetTimeout sets the connection's network
write timeout in nanoseconds.
SetWriteTimeout

[ukai, 2010-01-28 04:36:08]

LGTM.

Thanks for the fix.

http://codereview.appspot.com/196044/diff/8/1004
File src/pkg/websocket/websocket.go (right):

http://codereview.appspot.com/196044/diff/8/1004#newcode114
src/pkg/websocket/websocket.go:114: // Write implements the io.Closer interface
for a Conn.
s/Write/Close/ ?

http://codereview.appspot.com/196044/diff/8/1004#newcode117
src/pkg/websocket/websocket.go:117: // LocalAddr returns the local network
address for a Web Socket connection.
might be better to mention that it is WebSocket-Origin, not local address as of
socket.

Anyway, I'm thinking LocalAddr()/RemoteAddr() should be fixed as follows:
In client, local address is WebSocket-origin, and remote address is
WebSocket-location.
In server, local address is WebSocket-location, and remote address is
WebSocket-origin.

http://codereview.appspot.com/196044/diff/8/1004#newcode131
src/pkg/websocket/websocket.go:131: // SetTimeout sets the connection's network
read timeout in nanoseconds.
SetReadTimeout

http://codereview.appspot.com/196044/diff/8/1004#newcode139
src/pkg/websocket/websocket.go:139: // SetTimeout sets the connection's network
write timeout in nanoseconds.
SetWriteTimeout

[r, 2010-01-28 04:38:36]

*** Submitted as http://code.google.com/p/go/source/detail?r=e7b839fc365d ***

Regularize the comments for the websocket package and document all functions and
methods.

R=rsc, ukai
CC=golang-dev
http://codereview.appspot.com/196044

[r, 2010-01-28 04:39:10]

> Anyway, I'm thinking LocalAddr()/RemoteAddr() should be fixed as follows:
> In client, local address is WebSocket-origin, and remote address is
> WebSocket-location.
> In server, local address is WebSocket-location, and remote address is
> WebSocket-origin.

i'll leave this to you. this was a semantics-free change list.

[ukai, 2010-01-28 04:41:07]

On 2010/01/28 04:39:10, r wrote:
> > Anyway, I'm thinking LocalAddr()/RemoteAddr() should be fixed as follows:
> > In client, local address is WebSocket-origin, and remote address is
> > WebSocket-location.
> > In server, local address is WebSocket-location, and remote address is
> > WebSocket-origin.
> 
> i'll leave this to you. this was a semantics-free change list.

Ok, I'll create a change to fix it.