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

[WIP] core: Use AtomWatcher on macOS #1677

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

[WIP] core: Use AtomWatcher on macOS #1677

wants to merge 6 commits into from

Conversation

sebn
Copy link
Contributor

@sebn sebn commented Jul 12, 2019

So we can:

  • Drop the whole chokidar namespace (sort of)
  • Get rid of the 10s delay
  • Lower resources consumption

  • PR is not too big
  • it improves UX & DX in some way
  • it includes unit tests matching the implementation changes
  • it includes scenarios matching a new behaviour or has been manually tested
  • it includes relevant documentation

sebn added 6 commits July 12, 2019 13:55
We probably don't want to do this at first.
Just merge macOS support into master, so we can publish a new release.
And test the new combo by enabling it in config file.
Temporary proof-of-concept.
To be reimplemented as a macOS specific step just after producer.
(captures would take the output of this new step)
const timeoutInMs = process.env.NODE_ENV === 'test' ? 1000 : 10000
this.macOSBuffer = new LocalEventBuffer(timeoutInMs, batches => {
for (const batch of batches) {
this.process(batch)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

await?

this.watcher = await watcher.watchPath(
this.syncPath,
{ recursive: true },
this.process
onEventBatch
Copy link
Contributor Author

@sebn sebn Jul 12, 2019

Choose a reason for hiding this comment

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

should we regenerate all captures with buffer? (I believe they were regenerated before reintroducing the buffer)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also is the buffer necessary or not?

@taratatach taratatach marked this pull request as draft August 27, 2020 09:15
@taratatach taratatach force-pushed the master branch 2 times, most recently from 8a36ecb to a9a996e Compare April 10, 2021 09:52
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.

None yet

1 participant