FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

lib: replace legacy uses of __defineGetter__ by jasnell · Pull Request #6768 · nodejs/node · GitHub

/ node Public

lib: replace legacy uses of __defineGetter__ - #6768

Closed
jasnell wants to merge 1 commit into
nodejs:masterfrom
jasnell:remove-legacy-getter
ConversationCommits (1)ChecksFiles changed
Closed

lib: replace legacy uses of __defineGetter__#6768
jasnell wants to merge 1 commit into
nodejs:masterfrom
jasnell:remove-legacy-getter

Conversation

jasnell commented May 15, 2016 •
edited
Loading

Copy link
Copy Markdown
Member
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.

lib: replace legacy uses of __defineGetter__
d391be1 
Minor clean up. There are still some places in core that use
the legacy __defineGetter__ syntax. This updates those.
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 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 commented May 15, 2016

Copy link
Copy Markdown
Member Author

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

Trott mentioned this pull request May 15, 2016
Closed
2 tasks

Trott commented May 15, 2016

Copy link
Copy Markdown
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 commented May 15, 2016

Copy link
Copy Markdown
Member Author

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

jasnell commented May 16, 2016

Copy link
Copy Markdown
Member Author

@Trott ... does this PR LGTY?

jasnell commented May 16, 2016

Copy link
Copy Markdown
Member Author

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

Trott commented May 16, 2016

Copy link
Copy Markdown
Member

LGTM

jasnell added a commit that referenced this pull request May 17, 2016
lib: replace legacy uses of __defineGetter__
f293d0b 
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 commented May 17, 2016

Copy link
Copy Markdown
Member Author

Landed in f293d0b

jasnell closed this May 17, 2016
evanlucas pushed a commit that referenced this pull request May 17, 2016
lib: replace legacy uses of __defineGetter__
74582aa 
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>
evanlucas mentioned this pull request May 17, 2016
Merged
Trott added a commit to Trott/io.js that referenced this pull request May 18, 2016
tools: disallow deprecated define getter/setter
9ea72f0 
Refs: nodejs#6768
Trott added a commit to Trott/io.js that referenced this pull request May 20, 2016
tools: disallow deprecated define getter/setter
21b53fe 
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
tools: disallow deprecated define getter/setter
eeb0b9e 
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 commented Jun 2, 2016

Copy link
Copy Markdown
Contributor

@jasnell lts?

rvagg pushed a commit that referenced this pull request Jun 2, 2016
tools: disallow deprecated define getter/setter
ceee56b 
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 commented Jun 2, 2016

Copy link
Copy Markdown
Contributor

@thealphanerd Little value; I wouldn't bother.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL