Skip to content

doc: Provide bad/better examples for access() and exists() - #7832

Closed
dfabulich wants to merge 1 commit into
nodejs:masterfrom
dfabulich:exists-access-examples
Closed

doc: Provide bad/better examples for access() and exists()#7832
dfabulich wants to merge 1 commit into
nodejs:masterfrom
dfabulich:exists-access-examples

Conversation

@dfabulich

@dfabulichdfabulich commented Jul 22, 2016•
edited
Loading

Copy link
Copy Markdown
Contributor
Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change

@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 Jul 22, 2016
@dfabulichdfabulich mentioned this pull request Jul 22, 2016
Closed
4 tasks
jasnell
jasnell reviewed Aug 5, 2016
View reviewed changes
Comment threaddoc/api/fs.md Outdated

@jasnelljasnellAug 5, 2016

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.

As opposed to saying should not be used, perhaps something a bit softer...

Using `fs.acess()` to check for the existence of a file before calling `fs.open()`,
`fs.readFile()`, or `fs.writeFile()` is not recommended because doing so...

@jasnell

jasnell commented Aug 5, 2016

Copy link
Copy Markdown
Member

Thank you for this, left some comments! Sorry it has take so long for someone to take a look and get back to you.

@dfabulich
dfabulichforce-pushed the exists-access-examples branch from c3c11b3 to ccec465CompareAugust 9, 2016 18:51
@dfabulich

dfabulich commented Aug 9, 2016

Copy link
Copy Markdown
ContributorAuthor

@jasnell Thanks for your feedback. I've incorporated your comments in the latest draft.

@jasnell

jasnell commented Aug 11, 2016

Copy link
Copy Markdown
Member

@dfabulich .. thank you! will be able to go through in detail again tomorrow :-)

@Fishrock123

Fishrock123 commented Aug 22, 2016

Copy link
Copy Markdown
Contributor

ping @jasnell

@jasnell

jasnell commented Aug 22, 2016

Copy link
Copy Markdown
Member

Whoops, let this one slip off my radar. LGTM

@jasnell

jasnell commented Aug 24, 2016

Copy link
Copy Markdown
Member

ping @nodejs/documentation

addaleax
addaleax reviewed Aug 25, 2016
View reviewed changes
Comment threaddoc/api/fs.md Outdated

@addaleaxaddaleaxAug 25, 2016

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.

Maybe split the code blocks and put these pseudo-headings in bold just above them?

@addaleax

addaleax commented Aug 25, 2016

Copy link
Copy Markdown
Member

LGTM with a suggestion, thanks for putting these together!

thefourtheye
thefourtheye reviewed Aug 25, 2016
View reviewed changes
Comment threaddoc/api/fs.md Outdated

@thefourtheyethefourtheyeAug 25, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Drop the else, we can unindent the next function call.

@dfabulich
dfabulichforce-pushed the exists-access-examples branch from ccec465 to 80d6ec3CompareAugust 31, 2016 23:50
@dfabulich

dfabulich commented Aug 31, 2016

Copy link
Copy Markdown
ContributorAuthor

Incorporated feedback from @addaleax and @thefourtheye

@jasnell

jasnell commented Sep 1, 2016

Copy link
Copy Markdown
Member

LGTM! will get this landed!

jasnell pushed a commit that referenced this pull request Sep 1, 2016
@dfabulich@jasnell
doc: bad/better examples for fs.access() and fs.exists()
143d38c
PR-URL: #7832
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@jasnell

jasnell commented Sep 1, 2016

Copy link
Copy Markdown
Member

Landed in 143d38c. Thank you!

@jasnelljasnell closed this Sep 1, 2016
@Fishrock123Fishrock123 mentioned this pull request Sep 6, 2016
Closed
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Sep 8, 2016
@dfabulich@Fishrock123
doc: bad/better examples for fs.access() and fs.exists()
c0d828e
PR-URL: nodejs#7832
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Fishrock123 pushed a commit that referenced this pull request Sep 9, 2016
@dfabulich@Fishrock123
doc: bad/better examples for fs.access() and fs.exists()
372e4f3
PR-URL: #7832
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this pull request Sep 30, 2016
@dfabulichMyles Borins
doc: bad/better examples for fs.access() and fs.exists()
5b36210
PR-URL: #7832
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@dfabulich@jasnell@Fishrock123@addaleax@MylesBorins@thefourtheye@nodejs-github-bot