code review 123580043: sync: be more explicit in WaitGroup docs

sync: be more explicit in WaitGroup docs

Fixes issue 8543.

[adg, 2014-08-18 02:07:55]

Hello golang-codereviews@googlegroups.com,

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

[dvyukov, 2014-08-18 11:43:42]

LGTM content-wise, please wait for rsc/r for wording

Some of the Add's (let's call them "secondary") definitely can happen
concurrently with/after Wait.

[adg, 2014-08-20 07:23:59]

R=rsc,r

[r, 2014-08-20 14:11:47]

LGTM after tweaks

https://codereview.appspot.com/123580043/diff/1/src/pkg/sync/waitgroup.go
File src/pkg/sync/waitgroup.go (right):

https://codereview.appspot.com/123580043/diff/1/src/pkg/sync/waitgroup.go#new...
src/pkg/sync/waitgroup.go:42: // positive delta that start when the counter is
>0, may happen at any time.
i don't like the >0. although we all know what it means, it's ugly.

you actually don't need the first one:

Note that calls with a positive delta that occur when the counter is zero must
happen before a Wait.

And then you can write out the second:

when the counter is greater than zero

[adg, 2014-08-21 23:53:54]

*** Submitted as https://code.google.com/p/go/source/detail?r=0bb034265faf ***

sync: be more explicit in WaitGroup docs

Fixes issue 8543.

LGTM=dvyukov, r
R=rsc, r
CC=dvyukov, golang-codereviews
https://codereview.appspot.com/123580043

https://codereview.appspot.com/123580043/diff/1/src/pkg/sync/waitgroup.go
File src/pkg/sync/waitgroup.go (right):

https://codereview.appspot.com/123580043/diff/1/src/pkg/sync/waitgroup.go#new...
src/pkg/sync/waitgroup.go:42: // positive delta that start when the counter is
>0, may happen at any time.
On 2014/08/20 14:11:47, r wrote:
> i don't like the >0. although we all know what it means, it's ugly.
> 
> you actually don't need the first one:
> 
> Note that calls with a positive delta that occur when the counter is zero must
> happen before a Wait.
> 
> And then you can write out the second:
> 
> when the counter is greater than zero

Done.

[gobot, 2014-08-22 01:32:37]

This CL appears to have broken the darwin-386-cheney builder.
See http://build.golang.org/log/85c06000c6b389f53940d528eb658a8e99826405

[dave_cheney.net, 2014-08-22 02:03:55]

Looks like a bug in the test

ok mime/multipart 0.351s --- FAIL: TestErrorNil (0.00s) net_test.go:230:
Dial 127.0.0.1:65535 succeeded FAIL FAIL net 2.460s ok net/http

Isn't port 65535 magic in a similar way to port 0 ?
On 22 Aug 2014 11:32, <gobot@golang.org> wrote:

> This CL appears to have broken the darwin-386-cheney builder.
> See http://build.golang.org/log/85c06000c6b389f53940d528eb658a8e99826405
>
> https://codereview.appspot.com/123580043/
>
> --
> 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.
>

[rsc, 2014-08-22 14:52:35]

On Thu, Aug 21, 2014 at 10:03 PM, Dave Cheney <dave@cheney.net> wrote:

> Looks like a bug in the test
>
> ok mime/multipart 0.351s --- FAIL: TestErrorNil (0.00s) net_test.go:230:
> Dial 127.0.0.1:65535 succeeded FAIL FAIL net 2.460s ok net/http
>
> Isn't port 65535 magic in a similar way to port 0 ?
>

Not that I know of. Either way, it's the operating system that gave us that
port.

Russ