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

Fix constant in adapter to match with DELETE events #58

Merged
merged 1 commit into from
Aug 25, 2020

Conversation

XaF
Copy link
Contributor

@XaF XaF commented Jul 29, 2018

The previous behavior was duplicating a block of code and making
one of the two useless. This fixes that by replacing the constant to
IN_DELETE instead of IN_MOVED_FROM in one of the two blocks.

Signed-off-by: Raphaël Beamonte raphael.beamonte@gmail.com

The previous behavior was duplicating a block of code and making
one of the two useless. This fixes that by replacing the constant to
IN_DELETE instead of IN_MOVED_FROM in one of the two blocks.

Signed-off-by: Raphaël Beamonte <raphael.beamonte@gmail.com>
@XaF
Copy link
Contributor Author

XaF commented Jul 29, 2018

Please note that failing tests are not related...

@Elias481
Copy link

Again, Your fix is right - but sourrunding fixes need to be applied at same time otherwise test will fail.
See also my comment at #57 .
But for these errors You just need the fix from #57 (or a different one that I need but that fixes the same issue btw... that#s the reason I'm in to this..) to have the superficial argument promoted (=used for decision whether watch handle should be removed from inotify in the fix I have in mind). Otherwise the remove_watch raises Exception in trying to already automatically removed directory from inotify.
So You should probably put them into one merge request (including the missing fix to get #57 to not break anything but fix issues as #51 and #46 ).

Copy link
Owner

@dsoprea dsoprea left a comment

Choose a reason for hiding this comment

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

Excellent. Thanks. If you can, please introduce a test for this. Merging for now.

@dsoprea dsoprea merged commit 9be6a51 into dsoprea:master Aug 25, 2020
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

3 participants