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

Jump to bottom

doc: document fs.write limitation with TTY #24571

Closed
wants to merge 3 commits into from
Closed

doc: document fs.write limitation with TTY #24571

wants to merge 3 commits into from

Conversation

mcollina
Copy link
Member

Fixes: #24550

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

@mcollina build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1677/pipeline

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. labels Nov 22, 2018
@mcollina
Copy link
Member Author

cc @refack @nodejs/platform-windows

sam-github
sam-github reviewed Nov 22, 2018
View reviewed changes
doc/api/fs.md Outdated Show resolved Hide resolved
doc/api/fs.md Outdated Show resolved Hide resolved
refack
refack approved these changes Nov 22, 2018
View reviewed changes
Copy link
Contributor

@refack refack left a comment

Choose a reason for hiding this comment

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

LGTM in general.
I'm rubber-stamping any future phrasing changes.

doc/api/fs.md Outdated Show resolved Hide resolved
doc/api/fs.md Outdated Show resolved Hide resolved
doc/api/fs.md Outdated Show resolved Hide resolved
@mcollina
Copy link
Member Author

Updated with @refack changes (edited).
@sam-github please check it out.

refack
refack approved these changes Nov 22, 2018
View reviewed changes
Copy link
Contributor

@refack refack left a comment

Choose a reason for hiding this comment

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

💯

@refack
Copy link
Contributor

refack commented Nov 22, 2018

BTW: Is codepage 65001 as UTF-8, is common in POSIX land?

@mcollina
Copy link
Member Author

BTW: Is codepage 65001 as UTF-8, is common in POSIX land?

I never experienced this problem in Linux or Mac.

seishun
seishun reviewed Nov 22, 2018
View reviewed changes
doc/api/fs.md Show resolved Hide resolved
@mcollina
Copy link
Member Author

@seishun PTAL

mhdawson
mhdawson approved these changes Nov 22, 2018
View reviewed changes
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM once other peoples comments are addressed

@mcollina
Copy link
Member Author

I think this should be ready to land after the 48 expires.

@mcollina mcollina added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 23, 2018
@mcollina
Copy link
Member Author

CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/1683/

addaleax
addaleax reviewed Nov 24, 2018
View reviewed changes
doc/api/fs.md Outdated Show resolved Hide resolved
@addaleax @mcollina
Update doc/api/fs.md
aa1e3f4 
Co-Authored-By: mcollina <[email protected]>
seishun
seishun reviewed Nov 24, 2018
View reviewed changes
doc/api/fs.md Show resolved Hide resolved
seishun
seishun reviewed Nov 24, 2018
View reviewed changes
doc/api/fs.md Outdated Show resolved Hide resolved
@seishun @mcollina
Update doc/api/fs.md
85f4a5b 
Co-Authored-By: mcollina <[email protected]>
@mcollina
Copy link
Member Author

I'm going to land this tomorrow morning CET.

@Trott
Copy link
Member

Trott commented Nov 28, 2018

Lite CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/1703/

@Trott
Copy link
Member

Trott commented Nov 28, 2018

Landed in 086482d

@Trott Trott closed this Nov 28, 2018
Trott pushed a commit to Trott/io.js that referenced this pull request Nov 28, 2018
@mcollina @Trott
doc: document fs.write limitation with TTY
086482d 
Fixes: nodejs#24550

PR-URL: nodejs#24571
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Bartosz Sosnowski <[email protected]>
targos pushed a commit that referenced this pull request Nov 28, 2018
@mcollina @targos
doc: document fs.write limitation with TTY
2ebb32b 
Fixes: #24550

PR-URL: #24571
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Bartosz Sosnowski <[email protected]>
@BridgeAR BridgeAR mentioned this pull request Dec 5, 2018
Merged
4 tasks
This was referenced Dec 7, 2018
Closed
Closed
Closed
Closed
Closed
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
@mcollina @refack
doc: document fs.write limitation with TTY
cd97c5f 
Fixes: nodejs#24550

PR-URL: nodejs#24571
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Bartosz Sosnowski <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 12, 2019
@mcollina @BethGriggs
doc: document fs.write limitation with TTY
cfd5773 
Fixes: #24550

PR-URL: #24571
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Bartosz Sosnowski <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Feb 12, 2019
Merged
BethGriggs pushed a commit that referenced this pull request Feb 20, 2019
@mcollina @BethGriggs
doc: document fs.write limitation with TTY
dbc01e1 
Fixes: #24550

PR-URL: #24571
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Bartosz Sosnowski <[email protected]>
rvagg pushed a commit that referenced this pull request Feb 28, 2019
@mcollina @rvagg
doc: document fs.write limitation with TTY
4bc212d 
Fixes: #24550

PR-URL: #24571
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Bartosz Sosnowski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vsemozhetbyt vsemozhetbyt vsemozhetbyt left review comments

@seishun seishun seishun left review comments

@addaleax addaleax addaleax left review comments

@refack refack refack approved these changes

@sam-github sam-github sam-github left review comments

@bzoz bzoz bzoz approved these changes

@mhdawson mhdawson mhdawson approved these changes

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations. fs Issues 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.