Skip to content

Comments

fix: make PathsMut::commit consuming#695

Merged
JohnTitor merged 1 commit intonotify-rs:mainfrom
branchseer:fix/consuming-commit
Jul 2, 2025
Merged

fix: make PathsMut::commit consuming#695
JohnTitor merged 1 commit intonotify-rs:mainfrom
branchseer:fix/consuming-commit

Conversation

@branchseer
Copy link
Contributor

Ensures PathsMut::commit can only be called once, and adds more docs.
fixes concerns at #692 (comment)

/// Ensure added/removed paths are applied.
///
/// The behaviour of dropping a [`PathsMut`] without calling [`commit`] is unspecified.
/// The implementation is free to ignore the changes or not, and may leave the watcher in a started or stopped state.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't want the FsEventPathsMut to restart the watcher on drop because we can't return errors from drop. I think it's better to leave the state unspecified.

@TimNN
Copy link

TimNN commented Jul 2, 2025

Thanks @branchseer, this addresses all the concerns I had with the original implementation!

Copy link
Member

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

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

Thank you for a quick patch! And @TimNN thank you for raising concerns!

@JohnTitor JohnTitor merged commit 12a026d into notify-rs:main Jul 2, 2025
17 checks passed
JohnTitor pushed a commit that referenced this pull request Jul 3, 2025
JohnTitor pushed a commit that referenced this pull request Jan 16, 2026
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.

3 participants