Hello golang-dev@googlegroups.com, I'd like you to review this change to https://go.googlecode.com/hg/
LGTM http://codereview.appspot.com/5656048/diff/3021/src/pkg/net/dial_test.go File src/pkg/net/dial_test.go (right): http://codereview.appspot.com/5656048/diff/3021/src/pkg/net/dial_test.go#newc... src/pkg/net/dial_test.go:91: t.Logf("skipping test on %q; untested.", runtime.GOOS) untested? you tested it, and found that it hangs. I'd just say t.Logf("skipping known-broken test on windows") http://codereview.appspot.com/5656048/diff/3021/src/pkg/os/signal/signal_unix.go File src/pkg/os/signal/signal_unix.go (right): http://codereview.appspot.com/5656048/diff/3021/src/pkg/os/signal/signal_unix... src/pkg/os/signal/signal_unix.go:5: // +build darwin freebsd linux netbsd openbsd windows so much for this filename being _unix.go. :) http://codereview.appspot.com/5656048/diff/3021/src/pkg/os/signal/signal_wind... File src/pkg/os/signal/signal_windows_test.go (right): http://codereview.appspot.com/5656048/diff/3021/src/pkg/os/signal/signal_wind... src/pkg/os/signal/signal_windows_test.go:4: does _windows_test.go mean the same as _windows.go? I didn't know that, if so. If it doesn't, add a // build line. http://codereview.appspot.com/5656048/diff/3021/src/pkg/os/signal/signal_wind... src/pkg/os/signal/signal_windows_test.go:20: t.Fatalf("LoadDLL: %s\n", e) we tend to use %v for errors, instead of %s.
*** Submitted as http://code.google.com/p/go/source/detail?r=5ddbaedd8223 *** runtime, syscall, os/signal: fix windows build R=golang-dev, bradfitz CC=golang-dev http://codereview.appspot.com/5656048 http://codereview.appspot.com/5656048/diff/3021/src/pkg/net/dial_test.go File src/pkg/net/dial_test.go (right): http://codereview.appspot.com/5656048/diff/3021/src/pkg/net/dial_test.go#newc... src/pkg/net/dial_test.go:91: t.Logf("skipping test on %q; untested.", runtime.GOOS) On 2012/02/14 02:35:02, bradfitzgoog wrote: > untested? you tested it, and found that it hangs. > > I'd just say > > t.Logf("skipping known-broken test on windows") Done. http://codereview.appspot.com/5656048/diff/3021/src/pkg/os/signal/signal_unix.go File src/pkg/os/signal/signal_unix.go (right): http://codereview.appspot.com/5656048/diff/3021/src/pkg/os/signal/signal_unix... src/pkg/os/signal/signal_unix.go:5: // +build darwin freebsd linux netbsd openbsd windows On 2012/02/14 02:35:02, bradfitzgoog wrote: > so much for this filename being _unix.go. :) Agreed. I would have suggested to create a shortcut for "unix", but, unfortunately, the meaning of it varies sometimes. http://codereview.appspot.com/5656048/diff/3021/src/pkg/os/signal/signal_wind... File src/pkg/os/signal/signal_windows_test.go (right): http://codereview.appspot.com/5656048/diff/3021/src/pkg/os/signal/signal_wind... src/pkg/os/signal/signal_windows_test.go:4: On 2012/02/14 02:35:02, bradfitzgoog wrote: > does _windows_test.go mean the same as _windows.go? > Yes it does. http://codereview.appspot.com/5656048/diff/3021/src/pkg/os/signal/signal_wind... src/pkg/os/signal/signal_windows_test.go:20: t.Fatalf("LoadDLL: %s\n", e) On 2012/02/14 02:35:02, bradfitzgoog wrote: > we tend to use %v for errors, instead of %s. Done.
