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

Proposal: split Event structure into its own crate (notify-events?) #487

Closed
passcod opened this issue May 17, 2023 · 4 comments · Fixed by #559
Closed

Proposal: split Event structure into its own crate (notify-events?) #487

passcod opened this issue May 17, 2023 · 4 comments · Fixed by #559
Labels
A-enhancement Z-needs implementation Needs an implementation, will accept PRs

Comments

@passcod
Copy link
Member

passcod commented May 17, 2023

This is a proposal to only split out the notify/src/event.rs file into its own crate, which would have no required dependencies, and serde only when the feature is enabled.

The Event struct and child enums are pretty infrequently changed, and it would be helpful to downstream crates/users to be able to use the notify event types without pulling the entire notify runtime.

My specific motivation is for a usage across an IPC or network barrier, where one side does the file watching and the other wants to deserialize into the proper types.

@0xpr03
Copy link
Member

0xpr03 commented May 18, 2023

I don't see a problem with doing that. Some people will probably defend against a multi-crate setup, but if that helps, why not.

Only incoming breaking change would be one proposal for making #478 possible.

@0xpr03
Copy link
Member

0xpr03 commented May 18, 2023

Only bikeshed is the crate name. notify-events is fine-ish ? notify-types is probably more fitting

@0xpr03 0xpr03 added A-enhancement Z-needs implementation Needs an implementation, will accept PRs labels Jul 12, 2023
dfaust added a commit to dfaust/notify that referenced this issue Jan 25, 2024
0xpr03 pushed a commit that referenced this issue Jan 25, 2024
@dfaust
Copy link
Member

dfaust commented Jan 25, 2024

Oops, I just noticed that the debouncer crates have their own DebouncedEvent types.

What should we do about those?

  1. Create notify-debouncer-mini-types and notify-debouncer-full-types crates
  2. Move them into the notify-types crate
  3. Do nothing

@0xpr03
Copy link
Member

0xpr03 commented Jan 25, 2024

Option 2 should be fine. Not perfect but also it doesn't hurt anyone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-enhancement Z-needs implementation Needs an implementation, will accept PRs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants