code review 5483057: exp/winfsnotify: fix build.

exp/winfsnotify: fix build.

[mattn, 2011-12-13 11:41:37]

Hello golang-dev@googlecode.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
http://go.googlecode.com/hg/

[mattn, 2011-12-13 11:44:45]

http://codereview.appspot.com/5483057/diff/1002/src/pkg/exp/winfsnotify/winfs...
File src/pkg/exp/winfsnotify/winfsnotify.go (right):

http://codereview.appspot.com/5483057/diff/1002/src/pkg/exp/winfsnotify/winfs...
src/pkg/exp/winfsnotify/winfsnotify.go:350: e := syscall.CloseHandle(w.port)
Ah, it overwrite original err. will fix

[mattn, 2011-12-13 11:46:40]

http://codereview.appspot.com/5483057/diff/1002/src/pkg/exp/winfsnotify/winfs...
File src/pkg/exp/winfsnotify/winfsnotify.go (right):

http://codereview.appspot.com/5483057/diff/1002/src/pkg/exp/winfsnotify/winfs...
src/pkg/exp/winfsnotify/winfsnotify.go:350: e := syscall.CloseHandle(w.port)
Sorry, I was confused. This is correct.

On 2011/12/13 11:44:45, mattn wrote:
> Ah, it overwrite original err. will fix

[brainman, 2011-12-13 23:37:17]

Mattn, good that you noticed that it is broken!

Russ, can we have this package compiled and tested during build somehow?

Alex

http://codereview.appspot.com/5483057/diff/1002/src/pkg/exp/winfsnotify/winfs...
File src/pkg/exp/winfsnotify/winfsnotify.go (left):

http://codereview.appspot.com/5483057/diff/1002/src/pkg/exp/winfsnotify/winfs...
src/pkg/exp/winfsnotify/winfsnotify.go:79: return nil,
os.NewSyscallError("CreateIoCompletionPort", e)
Please, leave os.NewSyscallError("CreateIoCompletionPort", e) in. When program
fails, it is helpful to know which API failed, then you can see where the
problem is. Otherwise we will see errors, like "Access denied". I agree that
your new code looks simpler, but, I feel, we need good error messages in this
package - when things will go wrong, it will be hard to find the culprit.

Same everywhere else.

[mattn, 2011-12-14 01:25:41]

Done

On 2011/12/13 23:37:17, brainman wrote:
> Mattn, good that you noticed that it is broken!
> 
> Russ, can we have this package compiled and tested during build somehow?
> 
> Alex
> 
>
http://codereview.appspot.com/5483057/diff/1002/src/pkg/exp/winfsnotify/winfs...
> File src/pkg/exp/winfsnotify/winfsnotify.go (left):
> 
>
http://codereview.appspot.com/5483057/diff/1002/src/pkg/exp/winfsnotify/winfs...
> src/pkg/exp/winfsnotify/winfsnotify.go:79: return nil,
> os.NewSyscallError("CreateIoCompletionPort", e)
> Please, leave os.NewSyscallError("CreateIoCompletionPort", e) in. When program
> fails, it is helpful to know which API failed, then you can see where the
> problem is. Otherwise we will see errors, like "Access denied". I agree that
> your new code looks simpler, but, I feel, we need good error messages in this
> package - when things will go wrong, it will be hard to find the culprit.
> 
> Same everywhere else.

[brainman, 2011-12-14 02:17:02]

LGTM. Thank you.

[brainman, 2011-12-14 02:17:56]

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

exp/winfsnotify: fix build.

R=golang-dev, alex.brainman
CC=golang-dev
http://codereview.appspot.com/5483057

Committer: Alex Brainman <alex.brainman@gmail.com>