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.

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

lib: rename js source to lower snake_case #19556

Closed
wants to merge 3 commits into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Mar 23, 2018

This commit renames all JavaScript source files in lib to lower snake_case.

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

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. stream Issues and PRs related to the stream subsystem. labels Mar 23, 2018
@devsnek
Copy link
Member

devsnek commented Mar 23, 2018

in https://github.com/nodejs/node/tree/master/lib/internal/modules/esm everything is named in that style, i suppose they should also be renamed?

@TimothyGu
Copy link
Member

@vkurchatkin
Copy link
Contributor

It is named like this because it exports a single class.

@danbev
Copy link
Contributor Author

danbev commented Mar 25, 2018

It is named like this because it exports a single class.

I did notice this and that is why I brought it up. Do we want to keep it like this or should we rename all JavaScript source files to lowercase? (I'd prefer to rename)

@devsnek
Copy link
Member

devsnek commented Mar 25, 2018

i've had enough trouble with caseless filesystems to wholeheartedly recommend keeping everything as snake_case

@danbev
Copy link
Contributor Author

danbev commented Mar 26, 2018

@danbev
Copy link
Contributor Author

danbev commented Mar 26, 2018

node-test-commit-linuxone failure looks unrelated

console output:

Building addon /data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/addons-napi/test_function/
if [ -x /data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/./node ] && [ -e /data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/./node ]; then /data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/./node  tools/doc/generate.js --format=json doc/api/cluster.md > out/doc/api/cluster.json; elif [ -x `which node` ] && [ -e `which node` ] && [ `which node` ]; then `which node`  tools/doc/generate.js --format=json doc/api/cluster.md > out/doc/api/cluster.json; else echo "No available node, cannot run \"node  tools/doc/generate.js --format=json doc/api/cluster.md > out/doc/api/cluster.json\""; exit 1; fi;
  SOLINK_MODULE(target) Release/obj.target/binding.node
  COPY Release/binding.node
make[2]: Leaving directory `/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/addons/hello-world/build'
make[2]: write error
gyp ERR! build error 
gyp ERR! stack Error: `make` failed with exit code: 1
gyp ERR! stack     at ChildProcess.onExit (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/deps/npm/node_modules/node-gyp/lib/build.js:258:23)
gyp ERR! stackmake[1]: gyp info it worked if it ends with ok

@trivikr
Copy link
Member

trivikr commented Mar 28, 2018

This code looks good to land

@danbev danbev changed the title lib: rename BufferList.js to buffer_list.js lib: rename js source to lower snake_case Mar 28, 2018
@danbev
Copy link
Contributor Author

danbev commented Mar 28, 2018

Landed in f2b1079.

@danbev danbev closed this Mar 28, 2018
danbev added a commit that referenced this pull request Mar 28, 2018
This commit renames all JavaScript source files in lib to lower
snake_case.

PR-URL: #19556
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
@danbev danbev deleted the rename-BufferList branch March 28, 2018 06:31
@targos
Copy link
Member

targos commented Apr 2, 2018

Should this be backported to v9.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.

danbev added a commit to danbev/node that referenced this pull request Apr 30, 2018
This commit renames all JavaScript source files in lib to lower
snake_case.

PR-URL: nodejs#19556
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request May 1, 2018
This commit renames all JavaScript source files in lib to lower
snake_case.

PR-URL: nodejs#19556
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
targos pushed a commit that referenced this pull request Jun 6, 2018
This commit renames all JavaScript source files in lib to lower
snake_case.

Backport-PR-URL: #19969
PR-URL: #19556
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants