Skip to content

Fix code format in tests#617

Closed
tniessen wants to merge 1 commit intonodejs:masterfrom
tniessen:test-fix-formatting
Closed

Fix code format in tests#617
tniessen wants to merge 1 commit intonodejs:masterfrom
tniessen:test-fix-formatting

Conversation

@tniessen
Copy link
Copy Markdown
Member

@tniessen tniessen commented Dec 1, 2019

This fixes a few code formatting issues in test/.

@tniessen tniessen added the test label Dec 1, 2019
@legendecas
Copy link
Copy Markdown
Member

Maybe linters can be introduced to the project?

@tniessen
Copy link
Copy Markdown
Member Author

tniessen commented Dec 1, 2019

That is generally a good idea!

@NickNaso
Copy link
Copy Markdown
Member

NickNaso commented Dec 2, 2019

We could use clang-format. We can install it like dev dependencies. What do you think?

@tniessen
Copy link
Copy Markdown
Member Author

tniessen commented Dec 2, 2019

I have never used clang-format. Most Node.js projects use ESLint for the JS parts, and in core we use cpplint for C++. Seems like we also have a partial ruleset for clang-format, though.

Copy link
Copy Markdown
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mhdawson
Copy link
Copy Markdown
Member

mhdawson commented Dec 5, 2019

I think this can land while we figure out if we can add linters.

Copy link
Copy Markdown
Member

@legendecas legendecas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM either the linters could be added in the PR or another.

@gabrielschulhof
Copy link
Copy Markdown
Contributor

gabrielschulhof pushed a commit that referenced this pull request Dec 14, 2019
PR-URL: #617
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@gabrielschulhof
Copy link
Copy Markdown
Contributor

Landed in 0e7483e.

@tniessen tniessen deleted the test-fix-formatting branch January 23, 2020 03:47
kevindavies8 added a commit to kevindavies8/node-addon-api-Develop that referenced this pull request Aug 24, 2022
PR-URL: nodejs/node-addon-api#617
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Marlyfleitas added a commit to Marlyfleitas/node-api-addon-Development that referenced this pull request Aug 26, 2022
PR-URL: nodejs/node-addon-api#617
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
wroy7860 added a commit to wroy7860/addon-api-benchmark-node that referenced this pull request Sep 19, 2022
PR-URL: nodejs/node-addon-api#617
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this pull request Aug 11, 2023
PR-URL: nodejs/node-addon-api#617
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants