doc: note about non-existent files in watchFile

[thefourtheye]

Refer: #2093 (comment)

cc @brendanashworth @bnoordhuis

[thefourtheye]

Bump!

[silverwind]

Move that up to be included in above paragraph?

[thefourtheye]

@brendanashworth @silverwind I incorporated the review comments. PTAL now :-)

[brendanashworth]

Since two stat objects is redundant - it always does two stat objects - you could change it to something like invoke the listener once, with all fields zeroed (or, for dates, the Unix Epoch).

[thefourtheye]

@brendanashworth Thanks :-) Updated. PTAL now.

[brendanashworth]

LGTM. Did you want to add a test for this here?

[thefourtheye]

@brendanashworth Oh yeah, Thanks for pointing out :-) I included a test case now. PTAL.

[thefourtheye]

CI Run: iojs+any-pr+multi/217

[thefourtheye]

@bnoordhuis Looks like, in Windows blocks and blksize fields are undefined instead of zero. Shall I specialize those fields for Windows in test and doc?

[bnoordhuis]

Oh, that sounds right. Yes, special-casing for Windows is okay.

[thefourtheye]

New CI run with Windows specific changes: iojs+any-pr+multi/219. Ubuntu 15.04_64 box is failing without testing. I ll start the CI run when it is up.

[brendanashworth]

LGTM if the CI is happy (+ squash)

[thefourtheye]

New CI run: node-test-commit/85

[thefourtheye]

The build is Green. @bnoordhuis LGTY?

[bnoordhuis]

That shouldn't matter because the prototypes are the same, I think?

[thefourtheye]

@bnoordhuis Actually we are comparing expected against curr and prev. expected is just an object literal. So, the prototypes will be different.

[bnoordhuis]

Can't you do const expected = new fs.Stats()?

[thefourtheye]

@bnoordhuis It will use undefined for all the values.

> new fs.Stats()
{ dev: undefined,
  mode: undefined,
  nlink: undefined,
  uid: undefined,
  gid: undefined,
  rdev: undefined,
  blksize: undefined,
  ino: undefined,
  size: undefined,
  blocks: undefined,
  atime: Invalid Date,
  mtime: Invalid Date,
  ctime: Invalid Date,
  birthtime: Invalid Date }

Otherwise, I should be doing something like this

> const sDate = Date.UTC(1970, 0, 1, 0, 0, 0);
undefined
> new fs.Stats(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, sDate, sDate, sDate, sDate);
{ dev: 0,
  mode: 0,
  nlink: 0,
  uid: 0,
  gid: 0,
  rdev: 0,
  blksize: 0,
  ino: 0,
  size: 0,
  blocks: 0,
  atime: Thu Jan 01 1970 05:30:00 GMT+0530 (IST),
  mtime: Thu Jan 01 1970 05:30:00 GMT+0530 (IST),
  ctime: Thu Jan 01 1970 05:30:00 GMT+0530 (IST),
  birthtime: Thu Jan 01 1970 05:30:00 GMT+0530 (IST) }

Shall I change it like this?

[bnoordhuis]

I took a shortcut there, sorry, but the second one is indeed what I had in mind.

[thefourtheye]

@bnoordhuis Cool, I updated the test like that. Thanks :-)

[thefourtheye]

@bnoordhuis I updated the test to use fs.Stats to create the expected object. PTAL.

@brendanashworth I rebased and it is a single commit now :-) I feel that it would be better if you reviewed it once again.

CI Run: node-test-pull-request/42/

[thefourtheye]

Build is green, except the linter. I ran make lint and it didn't complain.

[jbergstroem]

Linter back now. Run here: https://jenkins-iojs.nodesource.com/job/node-test-pull-request/43/

[thefourtheye]

Thanks @jbergstroem :-) Linter also has given Green signal and the test passes in all the machines :-)

[bnoordhuis]

LGTM with a nit about the commit log: the first line should be <= 50 characters.

Aside, I would reword this:

this patch improves the Note section in watchFile to clarify about the non-existent files.

To e.g. this:

this patch documents the behavior of calling fs.watchFile() with a path that does not yet exist.

[thefourtheye]

@bnoordhuis I am landing it now and I ll change it

[thefourtheye]

Thanks guys, landed at ed85c95.