Issue 100860043: code review 100860043: go.exp/fsnotify: remove current implementation of WatchFlags

Issue 100860043: code review 100860043: go.exp/fsnotify: remove current implementation of WatchFlags (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
9 years, 11 months ago by nathany

Modified:
9 years, 10 months ago

Reviewers:
mikio, rsc

CC:
golang-codereviews, mikio, rsc

Visibility:
Public.

Description

go.exp/fsnotify: remove current implementation of WatchFlags removes fsnFlags, purgeEvents and internalEvent channel The current implementation: * doesn't take advantage of OS for efficiency * no benefit over filtering events as received * extra bookkeeping and mutexes * no tests * not correctly implemented on Windows https://github.com/howeyc/fsnotify/issues/93#issuecomment-39285195

Patch Set 1 #

Patch Set 2 : diff -r 19114a3ee7d5 https://code.google.com/p/go.exp #

Patch Set 3 : diff -r 19114a3ee7d5 https://code.google.com/p/go.exp #

Total comments: 2

Patch Set 4 : diff -r 19114a3ee7d5 https://code.google.com/p/go.exp #

Created: 9 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+37 lines, -153 lines)			Patch
M	fsnotify/fsnotify.go	View	1	1 chunk	+0 lines, -64 lines	0 comments	Download
M	fsnotify/fsnotify_bsd.go	View	1	7 chunks	+3 lines, -27 lines	0 comments	Download
M	fsnotify/fsnotify_linux.go	View	1 2 3	7 chunks	+17 lines, -37 lines	0 comments	Download
M	fsnotify/fsnotify_windows.go	View	1 2 3	5 chunks	+17 lines, -25 lines	0 comments	Download

Messages

Total messages: 7

Expand All Messages | Collapse All Messages

nathany

Hello golang-codereviews@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go.exp

9 years, 10 months ago (2014-06-12 14:19:28 UTC) #3

mikio

lgtm https://codereview.appspot.com/100860043/diff/40001/fsnotify/fsnotify_linux.go File fsnotify/fsnotify_linux.go (right): https://codereview.appspot.com/100860043/diff/40001/fsnotify/fsnotify_linux.go#newcode5 fsnotify/fsnotify_linux.go:5: // +build linux you don't need this tag ...

9 years, 10 months ago (2014-06-20 05:09:11 UTC) #4

nathany

Hello golang-codereviews@googlegroups.com, mikioh.mikioh@gmail.com, rsc@golang.org (cc: golang-codereviews@googlegroups.com), Please take another look.

9 years, 10 months ago (2014-06-20 05:32:18 UTC) #6

mikio

9 years, 10 months ago (2014-06-20 05:46:01 UTC) #7

*** Submitted as
https://code.google.com/p/go/source/detail?r=93897be904a1&repo=exp ***

go.exp/fsnotify: remove current implementation of WatchFlags

removes fsnFlags, purgeEvents and internalEvent channel

The current implementation:
* doesn't take advantage of OS for efficiency
* no benefit over filtering events as received
* extra bookkeeping and mutexes
* no tests
* not correctly implemented on Windows
https://github.com/howeyc/fsnotify/issues/93#issuecomment-39285195

LGTM=mikioh.mikioh, rsc
R=golang-codereviews, mikioh.mikioh, rsc
CC=golang-codereviews
https://codereview.appspot.com/100860043

Committer: Mikio Hara <mikioh.mikioh@gmail.com>

Expand All Messages | Collapse All Messages