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

tty: require readline at top of file #15647

Closed
wants to merge 1 commit into from
Closed

tty: require readline at top of file #15647

wants to merge 1 commit into from

Conversation

bengl
Copy link
Member

@bengl bengl commented Sep 28, 2017

No need to require it on each of those function calls.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

tty

@bengl
tty: require readline at top of file
8455230 
No need to require it on each of those function calls.
@nodejs-github-bot nodejs-github-bot added the tty Issues and PRs related to the tty subsystem. label Sep 28, 2017
@bengl
Copy link
Member Author

bengl commented Sep 28, 2017

CI: https://ci.nodejs.org/job/node-test-pull-request/10310/

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Sep 28, 2017 •
edited
Loading

Were they lazy loadings that became mandatory?

@bengl
Copy link
Member Author

bengl commented Sep 28, 2017

@vsemozhetbyt I doesn't seem to me like that's the case.

@BridgeAR
Copy link
Member

BridgeAR commented Oct 1, 2017

Landed in 200e783

@BridgeAR BridgeAR closed this Oct 1, 2017
BridgeAR pushed a commit that referenced this pull request Oct 1, 2017
@bengl @BridgeAR
tty: require readline at top of file
200e783 
No need to require it on each of those function calls.

PR-URL: #15647
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 3, 2017
@bengl @MylesBorins
tty: require readline at top of file
930809d 
No need to require it on each of those function calls.

PR-URL: #15647
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Oct 3, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Oct 3, 2017
@bengl @MylesBorins
tty: require readline at top of file
1f7c956 
No need to require it on each of those function calls.

PR-URL: #15647
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
addaleax pushed a commit to addaleax/ayo that referenced this pull request Oct 4, 2017
@bengl @addaleax
tty: require readline at top of file
b426dac 
No need to require it on each of those function calls.

PR-URL: nodejs/node#15647
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 11, 2017
@bengl @MylesBorins
tty: require readline at top of file
f68f572 
No need to require it on each of those function calls.

PR-URL: #15647
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
@MylesBorins
Copy link
Contributor

Should this be backported to v6.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

@bengl bengl mentioned this pull request Oct 17, 2017
Closed
MylesBorins pushed a commit that referenced this pull request Oct 18, 2017
@bengl @MylesBorins
tty: require readline at top of file
1d94a86 
No need to require it on each of those function calls.

Backport-PR-URL: #16264
PR-URL: #15647
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Oct 19, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
@bengl @MylesBorins
tty: require readline at top of file
88b9572 
No need to require it on each of those function calls.

Backport-PR-URL: #16264
PR-URL: #15647
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Nov 3, 2017
Merged
abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this pull request May 5, 2024
@bengl
tty: require readline at top of file
5dc4b25 
No need to require it on each of those function calls.

PR-URL: nodejs/node#15647
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@Fishrock123 Fishrock123 Fishrock123 approved these changes

@lpinca lpinca lpinca approved these changes

@cjihrig cjihrig cjihrig approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

Assignees
No one assigned
Labels
tty Issues and PRs related to the tty subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@bengl @vsemozhetbyt @BridgeAR @MylesBorins @jasnell @Fishrock123 @lpinca @cjihrig @nodejs-github-bot