Skip to content

lib: remove unnecessary assignment of exports - #20143

Closed
danbev wants to merge 1 commit into
nodejs:masterfrom
danbev:remove-unnecessary-exports-assignment
Closed

danbev wants to merge 1 commit into
nodejs:masterfrom
danbev:remove-unnecessary-exports-assignment

Conversation

@danbev

@danbev danbev commented Apr 19, 2018

Copy link
Copy Markdown
Contributor

This commit removes the assignment of exports since it is not used
in these files and there is no harm re-assigning module.exports.

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

This commit removes the assignment of exports since it is not used
in these files and there is no harm re-assigning module.exports.
@nodejs-github-bot nodejs-github-bot added errors Issues and PRs related to JavaScript errors originating in Node.js core. os Issues and PRs related to the os subsystem. labels Apr 19, 2018
@danbev

danbev commented Apr 19, 2018

Copy link
Copy Markdown
Contributor Author

@lpinca

lpinca commented Apr 19, 2018

Copy link
Copy Markdown
Member

there is no harm re-assigning module.exports

And it is reassigned anyway.

We are also documenting this pattern in https://nodejs.org/api/modules.html#modules_exports_shortcut. I honestly never liked it, I think it's confusing for newcomers.

@JacksonTian

Copy link
Copy Markdown
Contributor

I don't like this pattern too.

@trivikr

trivikr commented Apr 20, 2018

Copy link
Copy Markdown
Member

Re-run for CI node-test-commit-osx https://ci.nodejs.org/job/node-test-commit-osx/17983/

@BridgeAR BridgeAR added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Apr 22, 2018
@apapirovski

Copy link
Copy Markdown
Contributor

Landed in f31fc93

apapirovski pushed a commit that referenced this pull request Apr 22, 2018
This commit removes the assignment of exports since it is not used
in these files and there is no harm re-assigning module.exports.

PR-URL: #20143
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jackson Tian <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
jasnell pushed a commit that referenced this pull request Apr 23, 2018
This commit removes the assignment of exports since it is not used
in these files and there is no harm re-assigning module.exports.

PR-URL: #20143
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jackson Tian <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
@danbev
danbev deleted the remove-unnecessary-exports-assignment branch August 20, 2019 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. errors Issues and PRs related to JavaScript errors originating in Node.js core. os Issues and PRs related to the os subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants