|
|
|
go.exp: Add fsnotify package
Previous discussions:
https://codereview.appspot.com/5585043/
https://code.google.com/p/go/issues/detail?id=4068
Patch Set 1 #Patch Set 2 : diff -r 93e7901891c5 https://code.google.com/p/go.exp #Patch Set 3 : diff -r 93e7901891c5 https://code.google.com/p/go.exp #
Total comments: 3
Patch Set 4 : diff -r 93e7901891c5 https://code.google.com/p/go.exp #
Total comments: 1
Patch Set 5 : diff -r 93e7901891c5 https://code.google.com/p/go.exp #
Total messages: 9
|
howeyc
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go.exp
|
12 years, 10 months ago (2013年03月11日 17:10:44 UTC) #1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go.exp
Thanks. I'd like to try to make the API more uniform, so that it's hard to write code that accidentally uses a piece of the API that's only available on, say, Linux, but we can do that in followup CLs. https://codereview.appspot.com/7497045/diff/7001/fsnotify/example_test.go File fsnotify/example_test.go (right): https://codereview.appspot.com/7497045/diff/7001/fsnotify/example_test.go#new... fsnotify/example_test.go:1: package fsnotify_test insert copyright notice https://codereview.appspot.com/7497045/diff/7001/fsnotify/example_test.go#new... fsnotify/example_test.go:4: "code.google.com/p/go.exp/fsnotify" please use import ( "log" "code.google.com/p/go.exp/fsnotify" ) with a blank line separating standard imports from 'go get' imports. https://codereview.appspot.com/7497045/diff/7001/fsnotify/fsnotify_bsd.go File fsnotify/fsnotify_bsd.go (right): https://codereview.appspot.com/7497045/diff/7001/fsnotify/fsnotify_bsd.go#new... fsnotify/fsnotify_bsd.go:7: //Package fsnotify implements filesystem notification. better to move the doc comments into a single file that gets used always, like fsnotify.go, or else a new doc.go.
I've made changes based on your feedback.
https://codereview.appspot.com/7497045/diff/11001/fsnotify/fsnotify.go File fsnotify/fsnotify.go (right): https://codereview.appspot.com/7497045/diff/11001/fsnotify/fsnotify.go#newcode5 fsnotify/fsnotify.go:5: //Package fsnotify implements filesystem notification. please put a space after the // and we're good to go.
updated.
*** Submitted as https://code.google.com/p/go/source/detail?r=aa678ea78eb7&repo=exp *** go.exp: Add fsnotify package Previous discussions: https://codereview.appspot.com/5585043/ https://code.google.com/p/go/issues/detail?id=4068 R=golang-dev, rsc CC=golang-dev https://codereview.appspot.com/7497045 Committer: Russ Cox <rsc@golang.org>
LGTM I changed TestSubDir to skip the test, because it was failing on my Mac.
This change breaks go.exp repo windows build. This
exec.Command("mv", testFile, testFileRenamed)
(in test), will not work on windows. What is the plan here?
Thank you.
Alex