Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the fsnotify dependency from gopkg.in/fsnotify.v1 to the modern github.com/fsnotify/fsnotify v1.9.0 and sets up a proper Go module. The changes include updating API usage to match the newer fsnotify version and adding comprehensive logging for error handling.
- Updates fsnotify import from gopkg.in/fsnotify.v1 to github.com/fsnotify/fsnotify v1.9.0
- Modernizes API usage by replacing deprecated Op bitwise operations with Has() method calls
- Adds structured logging with slog for better error visibility and debugging
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| go.mod | Establishes Go module with fsnotify v1.9.0 dependency |
| rfsnotify.go | Updates fsnotify API usage and adds comprehensive error logging |
| rfsnotify_test.go | Adds example tests demonstrating recursive and non-recursive watching functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
rfsnotify.go
Outdated
| // Can't stat a deleted directory, so just pretend that it's always a directory and | ||
| // try to remove from the watch list... we really have no clue if it's a directory or not... | ||
| if e.Has(fsnotify.Remove) { | ||
| if er := m.fsnotify.Remove(e.Name); err != nil { |
There was a problem hiding this comment.
Variable name mismatch: the condition checks 'err' but should check 'er' which is the variable assigned from m.fsnotify.Remove().
| if er := m.fsnotify.Remove(e.Name); err != nil { | |
| if er := m.fsnotify.Remove(e.Name); er != nil { |
rfsnotify_test.go
Outdated
| return | ||
| } | ||
|
|
||
| // create a test file that will no trigger notify |
There was a problem hiding this comment.
Grammatical error: 'no' should be 'not' - 'create a test file that will not trigger notify'.
| // create a test file that will no trigger notify | |
| // create a test file that will not trigger notify |
rfsnotify_test.go
Outdated
| } | ||
| defer func() { | ||
| if e := f2.Close(); e != nil { | ||
| fmt.Printf("could not close file %s, error: %s", f.Name(), e) |
There was a problem hiding this comment.
Incorrect variable reference: should use 'f2.Name()' instead of 'f.Name()' since this defer block is for closing file 'f2'.
| fmt.Printf("could not close file %s, error: %s", f.Name(), e) | |
| fmt.Printf("could not close file %s, error: %s", f2.Name(), e) |
glimps-jbo
force-pushed
the
up_fsnotify
branch
2 times, most recently
from
4e6704f to
843f62f
Compare
glimps-jbo
force-pushed
the
up_fsnotify
branch
from
843f62f to
5c0f865
Compare
glimps-glv
force-pushed
the
up_fsnotify
branch
3 times, most recently
from
d5d7371 to
dc6453d
Compare
glimps-glv
force-pushed
the
up_fsnotify
branch
from
dc6453d to
f524c59
Compare
Hello,
Just a little PR to use updated version of fsnotify.
I set up a go module and do some light linting.
I also added a very basic example test.
Thanks for the lib.