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

fixup optional crossbeam feature selection in debouncer-mini #429

Merged
merged 1 commit into from Aug 16, 2022

Conversation

0xpr03
Copy link
Member

@0xpr03 0xpr03 commented Aug 14, 2022

Actually pass through the optional crossbeam dep from debouncer-mini to notify.

Also verify this in CI. (Requires a manual -p call to work.)

Also closes #433

@0xpr03 0xpr03 force-pushed the fixup_notify_feature branch 2 times, most recently from 170c2e0 to bc5d0ad Compare August 14, 2022 13:16
@0xpr03
Copy link
Member Author

0xpr03 commented Aug 14, 2022

The bad part: if we want to name the feature crossbeam-channel, as used in notify, we'll have to raise the MSRV to 1.60.
So this is my workaround.

@0xpr03
Copy link
Member Author

0xpr03 commented Aug 14, 2022

CC @JohnTitor for pro/contra

default = ["crossbeam-channel"]
default = ["crossbeam"]
# can't use dep:crossbeam-channel and feature name crossbeam-channel below rust 1.60
crossbeam = ["crossbeam-channel","notify/crossbeam-channel"]
Copy link
Member

Choose a reason for hiding this comment

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

Another way would be renaming our dependency like:

[dependencies]
crossbeam = { package = "crossbeam-channel", version = "..." }

Copy link
Member

Choose a reason for hiding this comment

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

But the crossbeam feature name doesn't sound that bad, so I'm fine either way :)

Copy link
Member Author

Choose a reason for hiding this comment

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

oh didn't think of renaming them, but yeah I'll rename the feature in the debouncer

@0xpr03 0xpr03 force-pushed the fixup_notify_feature branch 4 times, most recently from 7673bd0 to ba67eae Compare August 16, 2022 22:40
@0xpr03 0xpr03 merged commit 54465e9 into main Aug 16, 2022
@0xpr03 0xpr03 deleted the fixup_notify_feature branch August 16, 2022 22:58
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.

Reminder: the debouncer lib.rs still has debug println
2 participants