Skip to content

doc: clarify fs.utimes() arguments - #8651

Closed
dguo wants to merge 1 commit into
nodejs:masterfrom
dguo:8638-fs-utimes-docs
Closed

doc: clarify fs.utimes() arguments#8651
dguo wants to merge 1 commit into
nodejs:masterfrom
dguo:8638-fs-utimes-docs

Conversation

@dguo

@dguodguo commented Sep 18, 2016

Copy link
Copy Markdown
Contributor
Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
  • doc
Description of change

Make it clear that the atime and mtime arguments to fs.utimes() should be in seconds.

Fixes#8638

@nodejs-github-botnodejs-github-bot added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. labels Sep 18, 2016

@imyllerimyller left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@addaleaxaddaleax left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Comment threaddoc/api/fs.md Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: I realize it’s copied, but numerable sounds a bit weird to me… maybe just “numeric”?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Good point. I updated the commit with numeric.

Make it clear that atime and mtime should be in seconds.
@dguo
dguoforce-pushed the 8638-fs-utimes-docs branch from 2dc1298 to bd2f332CompareSeptember 19, 2016 12:55

@benjamingrbenjamingr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@jasnelljasnell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

jasnell pushed a commit that referenced this pull request Sep 20, 2016
Make it clear that atime and mtime should be in seconds.
PR-URL: #8651
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
@jasnell

Copy link
Copy Markdown
Member

Landed in 99ab686. Thank you!

@jasnelljasnell closed this Sep 20, 2016
Fishrock123 pushed a commit that referenced this pull request Oct 11, 2016
Make it clear that atime and mtime should be in seconds.
PR-URL: #8651
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
@dguo
dguo deleted the 8638-fs-utimes-docs branch November 30, 2016 21:19
@dguodguo mentioned this pull request Dec 1, 2016
1 task
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docIssues and PRs related to the documentations.fsIssues and PRs related to the fs subsystem / file system.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fs.utimes arguments documentation

8 participants

@dguo@jasnell@addaleax@benjamingr@imyller@not-an-aardvark@MylesBorins@nodejs-github-bot