Closed
Conversation
Those are run by golangci-lint, so we don't have to. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Fix these: inotify_test.go:134:7: S1005: unnecessary assignment to the blank identifier (gosimple) case _ = <-w.Events: ^ inotify_test.go:351:2: S1005: unnecessary assignment to the blank identifier (gosimple) _ = <-w.Events // consume Remove event ^ Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The staticcheck linter suggests: inotify_test.go:146:2: SA5001: should check returned error before deferring w.Close() (staticcheck) defer w.Close() ^ Instead, remove the defer entirely because we have one already earlier in the code. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The staticcheck linter warns:
inotify_poller.go:122:5: SA9003: empty branch (staticcheck)
if event.Events&unix.EPOLLHUP != 0 {
^
Commit 8165837 removed the code that handles this case,
so it is no longer needed. The comment would be nice to have, but it
is sort of obvious.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Check for and return an error in case w.poller.wake() failed, to fix this warning: inotify.go:83:15: Error return value of `w.poller.wake` is not checked (errcheck) w.poller.wake() ^ Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Add t.Helper() call to newWatcher() and addWatch() helper functions for tests to show proper file/line information on errors. 2. Fix addWatch argument name and docstring since it is not always a directory. 3. Use newWatcher and addWatch where possible, fixing a few cases of missing error checks. 4. Fix a case where os.Create() result is not checked, and the resulting file is not closed. 5. Some related refactoring. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Fix two cases where the test tries to write to a file which was opened read-only. 2. Add and use helper functions ok() and writeData() to aid in tests error checking. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
At the moment there should be none. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Closed
arp242
reviewed
Jul 21, 2022
| w.poller.wake() | ||
| if err := w.poller.wake(); err != nil { | ||
| return fmt.Errorf("poller.wake failed: %w", err) | ||
| } |
Member
There was a problem hiding this comment.
I'm not entirely sure if this is really the correct behaviour; I think that this failing isn't really a big deal in the Close() here?
arp242
reviewed
Jul 21, 2022
| if event.Events&unix.EPOLLHUP != 0 { | ||
| // Write pipe descriptor was closed, by us. This means we're closing down the | ||
| // watcher, and we should wake up. | ||
| } |
Member
There was a problem hiding this comment.
The comment says it "should wake up", but it doesn't do anything. I'm not sure if this can just be removed, or if there's an omission here and we should put something in this branch.
Member
|
Should be fixed in the main branch now. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this pull request do?
Simplifies
lintCI job by removinggo vetandgofmt(those are called by golangci-lint anyway).Fixes all warnings found by the default set of golangci-lint checkers. In some cases those are real bugs, so please review carefully.
Stop ignoring golangci-lint warnings.
Where should the reviewer start?
Please review commit by commit, noting the commit messages.
GOOS=darwin golangci-lint run) that I am afraid to tackle.How should this be manually tested?
No manual tests needed; this is why we have CI! 😉 👌🏻