Skip to content

doc: fix sentence fragment in fs doc #6488

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from
Closed

Conversation

Trott
Copy link
Member

@Trott Trott commented Apr 30, 2016

Checklist
  • documentation is changed or added
  • the commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change

Edit a sentence fragment so that it is a complete sentence.

@Trott Trott added the doc Issues and PRs related to the documentations. label Apr 30, 2016
@@ -797,8 +797,8 @@ the end of the file.

_Note: The behavior of `fs.open()` is platform specific for some flags. As such,
opening a directory on OS X and Linux with the `'a+'` flag - see example below -
will return an error. Whereas on Windows and FreeBSD a file descriptor will be
Copy link
Member

Choose a reason for hiding this comment

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

I'd just s/error. Whereas/error, whereas/

Copy link
Member Author

Choose a reason for hiding this comment

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

I considered that, but then you have a fairly long sentence that is probably harder to understand than the two separate sentences:

As such, opening a directory on OS X and Linux with the 'a+' flag - see example below - will return an error, whereas on Windows and FreeBSD a file descriptor will be returned.

I could be persuaded, but I do prefer splitting that into two sentences. The see example below in the middle of the long sentence is especially kind of begging for a shorter sentence.

Copy link
Member

Choose a reason for hiding this comment

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

either way works for me.

@jasnell
Copy link
Member

jasnell commented Apr 30, 2016

small nit, but LGTM

@benjamingr
Copy link
Member

LGTM

@mscdex mscdex added the fs Issues and PRs related to the fs subsystem / file system. label Apr 30, 2016
@estliberitas
Copy link
Contributor

LGTM

jasnell pushed a commit that referenced this pull request May 1, 2016
PR-URL: #6488
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Alexander Makarenko <estliberitas@gmail.com>
@jasnell
Copy link
Member

jasnell commented May 1, 2016

Landed in 9f8d0ea

@jasnell jasnell closed this May 1, 2016
Fishrock123 pushed a commit that referenced this pull request May 4, 2016
PR-URL: #6488
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Alexander Makarenko <estliberitas@gmail.com>
joelostrowski pushed a commit to joelostrowski/node that referenced this pull request May 4, 2016
PR-URL: nodejs#6488
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Alexander Makarenko <estliberitas@gmail.com>
@Trott Trott deleted the frag branch January 13, 2022 22:43
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants