Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a feature to return the directories and files that are being monitored #374

Merged
merged 4 commits into from Apr 21, 2022

Conversation

NitroCao
Copy link
Contributor

What does this pull request do?

Add a method for Watcher to return the directories and files that are being monitored.

Where should the reviewer start?

WatchList method in inotify.go.

How should this be manually tested?

unit testing in inotify_test.go.

PaluMacil
PaluMacil previously approved these changes Jul 29, 2021
Copy link

@PaluMacil PaluMacil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks appropriate and tested

Copy link
Contributor

@nathany nathany left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution.

Unfortunately we can't add a new WatchList() API that only works on Linux.

fsnotify is a cross-platform library. If we want to add WatchList(), it must also be added for macOS/BSD (kqueue) and Windows.

@mattn
Copy link
Member

mattn commented Jan 15, 2022

@NitroCao Would you update this?

inotify.go Show resolved Hide resolved
@chamabreu
Copy link

Hi, is this feature comming?

@NitroCao
Copy link
Contributor Author

Sorry for the long delay. Just added support for bsd and windows platforms.

kqueue.go Outdated Show resolved Hide resolved
windows.go Outdated Show resolved Hide resolved
@mattn
Copy link
Member

mattn commented Apr 19, 2022

@nathany do you have opinion others?

@nathany
Copy link
Contributor

nathany commented Apr 19, 2022

LGTM

@mattn
Copy link
Member

mattn commented Apr 20, 2022

Thanks. I want to confirm rules for mainteners. Only administrators have to merge this? (I know I can merge this)

image

@nathany
Copy link
Contributor

nathany commented Apr 21, 2022

Looks like there is a problem with Hound

@nathany
Copy link
Contributor

nathany commented Apr 21, 2022

I removed Hound and the branch protection rule that required it.

@NitroCao
Copy link
Contributor Author

So can we merge this PR now?

@mattn mattn merged commit ceba4ef into fsnotify:main Apr 21, 2022
@mattn
Copy link
Member

mattn commented Apr 21, 2022

Thank you

@NitroCao NitroCao deleted the watchlist branch April 21, 2022 07:52
@nathany
Copy link
Contributor

nathany commented Apr 21, 2022

thanks @mattn

I see there is a 1.5.3 tag that was created before the 1.5.2 tag for this. Will that cause problems getting this update?

@mattn
Copy link
Member

mattn commented Apr 22, 2022

OpenBSD build was broken since before. #443

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants