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

Disable @atom/notify on master for now #19345

Merged
merged 1 commit into from May 16, 2019

Conversation

@nathansobo
Copy link
Contributor

commented May 15, 2019

We're seeing some bad behavior from the Rust notify crate that has me worried about the entire direction proposed in #19244.

  1. I've personally seen at least one instance of the crate not picking up events from the file system.
  2. @as-cii has witnessed rogue instances notify-subprocess-darwin pegging a core at 100%. When we sampled them, it looked like they were in a busy loop in the unwatch method of the Notify crate.

Both cases undermine my confidence in the Notify crate. My premise was that we'd take a reliable solution and wrap it in a subprocess, but it's seeming like that solution isn't as battle-tested and solid as I had hoped. For now, we can switch back to @atom/nsfw while we figure out how to proceed from here.

cc @as-cii @rafeca

@nathansobo nathansobo merged commit 3d37651 into master May 16, 2019

2 checks passed

Atom Pull Requests #20190515.7 succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@nathansobo nathansobo deleted the ns/notify-retreat branch May 16, 2019

nathansobo added a commit that referenced this pull request May 17, 2019

Revert "Merge pull request #19345 from atom/ns/notify-retreat"
This reverts commit 3d37651, reversing
changes made to e1a5d52.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.