search

atom / watcher

Atom Filesystem Watcher
MIT License
200 stars 36 forks source link

Support node 12 #224

Closed caseywebdev closed 4 years ago

caseywebdev commented 5 years ago

Description of the Change

Add node 12 to the build systems

Benefits

Supports the latest version of node

benjamn commented 5 years ago

Thanks for working on this @caseywebdev! We're planning to switch from pathwatcher to @atom/watcher for Meteor 1.9, and Node.js 12 compatibility is the main remaining blocker.

smashwilson commented 5 years ago

Just so you're aware: CircleCI is currently failing because the version of clang-tidy on homebrew changed and inherited additional checks. So if you see that failing, it's not you 😅

nathansobo commented 5 years ago

Hey @benjamn, just a heads-up: Atom has been hitting some crashes on Windows CI that seem to be originating from @atom/watcher when we attempt to enable it by default. I'm currently exploring a transition to yet another solution which spawns a Rust subprocess based on the notify crate to do the watching and relays it to JS. This will live in the @atom/notify module.

It's still a bit early, but if I'm successful with this approach, we may not end up using watcher in Atom. I just wanted to let you know in case that influences your decision to depend on this module.

@smashwilson Feel free to chime in.

smashwilson commented 5 years ago

@caseywebdev on a related note, would you be interested in having push and merge rights on this repo and the npm package? That way you wouldn't have to be blocked on me :smile:

caseywebdev commented 5 years ago

I can't really commit time to maintaining it, so I'd have to respectfully decline. I can't even get this PR to pass 😉

taratatach commented 4 years ago

Hey @smashwilson, are you planning on working on this PR and merging it anytime soon? It's not possible to compile @atom/watcher on macOS with Electron 6 because of the nan version used in the latest release. If I can help in any way, please let me know.

smashwilson commented 4 years ago

I haven't been actively working on this module for some time, no.

If you're so inclined, though, you could start with this PR and get the builds green and I'd be happy to merge :smile:

DeeDeeG commented 4 years ago

For anyone reading this, Node 12 support appears to have landed with #234.