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.

Sign up for GitHub

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

doc: improve fs.utimes #14154

Closed
wants to merge 1 commit into from
Closed

doc: improve fs.utimes #14154

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 12 additions & 15 deletions doc/api/fs.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1219,12 +1219,12 @@ changes:
-->

* `fd` {integer}
* `atime` {integer}
* `mtime` {integer}
* `atime` {number|string|Date}
* `mtime` {number|string|Date}
* `callback` {Function}

Change the file timestamps of a file referenced by the supplied file
descriptor.
Change the file system timestamps of the object referenced by the supplied file
descriptor. See [`fs.utimes()`][].

*Note*: This function does not work on AIX versions before 7.1, it will return
the error `UV_ENOSYS`.
Expand Down Expand Up @@ -2211,20 +2211,17 @@ changes:
-->

* `path` {string|Buffer|URL}
* `atime` {integer}
* `mtime` {integer}
* `atime` {number|string|Date}
* `mtime` {number|string|Date}
* `callback` {Function}

Change file timestamps of the file referenced by the supplied path.

*Note*: The arguments `atime` and `mtime` of the following related functions
follow these rules:
Change the file system timestamps of the object referenced by `path`.

- The value should be a Unix timestamp in seconds. For example, `Date.now()`
returns milliseconds, so it should be divided by 1000 before passing it in.
- If the value is a numeric string like `'123456789'`, the value will get
converted to the corresponding number.
- If the value is `NaN`, `Infinity` or `-Infinity`, an Error will be thrown.
The `atime` and `mtime` arguments follow these rules:
- Values can be either numbers representing Unix epoch time, `Date`s, or a
numeric string like `'123456789.0'`.
- If the value can not be converted to a number, or is `NaN`, `Infinity` or
`-Infinity`, a `Error` will be thrown.
Copy link
Contributor

Choose a reason for hiding this comment

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

Still "an Error"?

Copy link
Member

Choose a reason for hiding this comment

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

What happens if Invalid Date is passed?

Also, if we can pass a string that gets coerced to numbers, then I'm not sure we should put string in the docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Still an Error in master - https://github.com/nodejs/node/blob/master/lib/fs.js#L1196
  2. Even a Date is converted (or coerced) to number so the sentence still stands true (Number(new Date("foo")) === NaN), just missing an explicit mention.

Copy link
Contributor

@vsemozhetbyt vsemozhetbyt Jul 10, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@refack Sorry, I meant a nit, still "an", not "a")

Copy link
Contributor Author

@refack refack Jul 10, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@benjamingr the string note was alway there. As for the arg type I think it's better to explicitly state string but this is JS1.0 semantics so we need to ask ourselves WWDCD?
What Would Douglas Crockford Do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@refack Sorry, I meant a nit, still "an", not "a")

Ack. But I'm not pushing a fix yet, so the string conversation will stay open.


## fs.utimesSync(path, atime, mtime)
<!-- YAML
Expand Down