Skip to content

Conversation

@jasnell
Copy link
Member

@jasnelljasnell 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.
@jasnelljasnell 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-botnodejs-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
MemberAuthor

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

@Trott
Copy link
Member

@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
MemberAuthor

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

@jasnell
Copy link
MemberAuthor

@Trott ... does this PR LGTY?

@jasnell
Copy link
MemberAuthor

@Trott
Copy link
Member

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 <[email protected]>
@jasnell
Copy link
MemberAuthor

Landed in f293d0b

@jasnelljasnell 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 <[email protected]>
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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Johan Bergström <[email protected]> Refs: nodejs#6768
Fishrock123 pushed a commit that referenced this pull request May 23, 2016
PR-URL: #6774 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Johan Bergström <[email protected]> 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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Johan Bergström <[email protected]> Refs: #6768
@Fishrock123
Copy link
Contributor

@thealphanerd Little value; I wouldn't bother.

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

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@jasnell@Trott@MylesBorins@Fishrock123@nodejs-github-bot