Skip to content

Conversation

@whatyoubendoing
Copy link

Use polling to monitor for changes. Omitting the interval will default
to 100ms. This option is useful if you're watching an NFS volume. Fixes #13

Use polling to monitor for changes. Omitting the interval will default
to 100ms. This option is useful if you're watching an NFS volume.
@danhayden
Copy link

This PR fixes an issue we are having on windows machines. Any update on if/when this will be merged?

opts.usePolling = true;
opts.interval = argv.poll !== true
? argv.poll
: undefined;
Copy link
Member

Choose a reason for hiding this comment

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

interval should be Number in the chokidar/lib/nodefs-handler.js#150. How about to update this part to the following?

if (typeof argv.poll === 'number') {
  opts.usePolling = true;
  opts.interval = argv.poll;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

@watilde Should be:

  if (argv.poll) {
    opts.usePolling = true;
  }
  if (typeof argv.poll === 'number') {
    opts.interval = argv.poll;
  }

So that:

Omitting the interval will default to 100ms.

See https://github.com/paulmillr/chokidar#performance.

@watilde
Copy link
Member

watilde commented May 7, 2016

Hello Benjamin! Thanks for the nice feature patch! It will be LGTM immediately when you update the code on the point I left a comment and add a test code :)

// and thanks for letting me know this! @danhayden

@RyanZim
Copy link
Collaborator

RyanZim commented Aug 18, 2016

@B3njamin ping?

@whatyoubendoing
Copy link
Author

Hey unfortunately 😢 I don't have time to fix the above or write any tests. Is someone else able to do this.

  • Ben

@RyanZim
Copy link
Collaborator

RyanZim commented Aug 18, 2016

I can fix the code, just not sure how to go about writing tests.

@watilde What did you have in mind for tests?

@watilde
Copy link
Member

watilde commented Aug 19, 2016

@B3njamin Sure thing! Take your time :)

@RyanZim Thank you so much for your help! Could you please cherry-pick his commit and recreate a pull request if you are available on it? For test: we can add new case that uses the poll option.

@RyanZim
Copy link
Collaborator

RyanZim commented Aug 19, 2016

Changes made at: https://github.com/RyanZim/postcss-cli/tree/poll. Need to write tests yet.

@RyanZim RyanZim mentioned this pull request Aug 20, 2016
@RyanZim
Copy link
Collaborator

RyanZim commented Aug 20, 2016

@B3njamin @watilde: PR made: #39.

@watilde
Copy link
Member

watilde commented Aug 29, 2016

This is shipped as 59e6621. Thank you so much putting this together!

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.

4 participants