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

test: common.js -> common #917

Closed
wants to merge 1 commit into from

Conversation

brendanashworth
Copy link
Contributor

This commit changes many test styles to change all references
from require('./common.js'); to require('./common');.

The latter is much more common, with the former only being used in 50
tests. It is just a stylistic change, and it seems that common.js was
introduced by a rogue test and copied and pasted into the rest.

This commit changes many test styles to change all references
from `require('./common.js');` to `require('./common');`.

The latter is much more common, with the former only being used in 50
tests. It is just a stylistic change, and it seems that `common.js` was
introduced by a rogue test and copied and pasted into the rest.
@cjihrig
Copy link
Contributor

cjihrig commented Feb 22, 2015

Purely style changes aren't typically allowed. I'm not opposed to this, but I'll leave it up to someone else to merge. The code looks fine FWIW.

@brendanashworth
Copy link
Contributor Author

It didn't take me much time so I'm okay either way. I made the changes
because whenever new tests and benchmarks are added with the .js style,
there isn't much grounds to ask them to change it to normal io.js code
guidelines because many other tests also follow that style.

Plus, they're always right beside require('assert') calls and
never require('assert.js'), which just... yeah.

@bnoordhuis
Copy link
Member

I'm fine with it. LGTM.

brendanashworth added a commit that referenced this pull request Feb 23, 2015
This commit changes many test styles to change all references
from require('./common.js'); to require('./common');.

The latter is much more common, with the former only being used in 50
tests. It is just a stylistic change, and it seems that `common.js` was
introduced by a rogue test and copied and pasted into the rest.

Semver: patch
PR-URL: #917
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@chrisdickinson
Copy link
Contributor

Merged in 0df5430. Thanks!

This was referenced Feb 23, 2015
@brendanashworth brendanashworth deleted the test-remove-js branch March 8, 2015 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants