Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@jasnell
Copy link
Member

@jasnell jasnell commented May 15, 2016

Checklist
  • tests and code linting passes
  • the commit message follows commit guidelines
Affected core subsystem(s)

crypto, readline, internal

Description of change

Minor clean up. There are still some places in core that use the legacy __defineGetter__ syntax. This updates those.

Minor clean up. There are still some places in core that use
the legacy __defineGetter__ syntax. This updates those.
@jasnell jasnell added crypto Issues and PRs related to the crypto subsystem. readline Issues and PRs related to the built-in readline module. labels May 15, 2016
@nodejs-github-bot nodejs-github-bot added tls Issues and PRs related to the tls subsystem. lib / src Issues and PRs related to general changes in the lib or src directory. labels May 15, 2016
@jasnell
Copy link
Member Author

jasnell commented May 15, 2016

@Trott ... is there an eslint rule that can catch these?

@Trott
Copy link
Member

Trott commented May 15, 2016

@Trott ... is there an eslint rule that can catch these?

Not a pre-existing one, but I've made a custom one: #6774

According to that lint rule, there are two files missed in this PR:

  • test/parallel/test-util-inspect.js
  • lib/internal/process/stdio.js

I can do them as part of that PR or you can add them to this one. Doesn't matter to me either way.

@jasnell
Copy link
Member Author

jasnell commented May 15, 2016

I have a separate pr that covers the stdio ones. I intentionally did not touch the tests.

@jasnell
Copy link
Member Author

jasnell commented May 16, 2016

@Trott ... does this PR LGTY?

@jasnell
Copy link
Member Author

jasnell commented May 16, 2016

@Trott
Copy link
Member

Trott commented May 16, 2016

LGTM

jasnell added a commit that referenced this pull request May 17, 2016
Minor clean up. There are still some places in core that use
the legacy __defineGetter__ syntax. This updates most of those.

PR-URL: #6768
Reviewed-By: Rich Trott <rtrott@gmail.com>
@jasnell
Copy link
Member Author

jasnell commented May 17, 2016

Landed in f293d0b

@jasnell jasnell closed this May 17, 2016
evanlucas pushed a commit that referenced this pull request May 17, 2016
Minor clean up. There are still some places in core that use
the legacy __defineGetter__ syntax. This updates most of those.

PR-URL: #6768
Reviewed-By: Rich Trott <rtrott@gmail.com>
Trott added a commit to Trott/io.js that referenced this pull request May 18, 2016
Trott added a commit to Trott/io.js that referenced this pull request May 20, 2016
PR-URL: nodejs#6774
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Refs: nodejs#6768
Fishrock123 pushed a commit that referenced this pull request May 23, 2016
PR-URL: #6774
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Refs: #6768
@MylesBorins
Copy link
Contributor

@jasnell lts?

rvagg pushed a commit that referenced this pull request Jun 2, 2016
PR-URL: #6774
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Refs: #6768
@Fishrock123
Copy link
Contributor

@thealphanerd Little value; I wouldn't bother.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

crypto Issues and PRs related to the crypto subsystem. lib / src Issues and PRs related to general changes in the lib or src directory. readline Issues and PRs related to the built-in readline module. tls Issues and PRs related to the tls subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.