-
Notifications
You must be signed in to change notification settings - Fork 30k
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
Replaced common.fixturesDir with usage of common.fixtures module #15840
Conversation
(First-time contribution during Node.js Interactive)
@@ -40,7 +41,7 @@ assert.strictEqual(baseBar, // eslint-disable-line no-undef | |||
'bar', | |||
'global.x -> x in base level not working'); | |||
|
|||
const mod = require(path.join(common.fixturesDir, 'global', 'plain')); | |||
const mod = require(path.join(fixtures.fixturesDir, 'global', 'plain')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be using method fixtures.path
instead of path.join
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pawelgolda is correct, the correct change here would be:
const mod = require(fixtures.path('global', 'plain'));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am going to fix this while landing.
Thank you so much for the contribution and for participating in the code-and-learn! There are a couple of things that need to be fixed here but it's very close! |
soft ping @tkarsten |
@@ -40,7 +41,7 @@ assert.strictEqual(baseBar, // eslint-disable-line no-undef | |||
'bar', | |||
'global.x -> x in base level not working'); | |||
|
|||
const mod = require(path.join(common.fixturesDir, 'global', 'plain')); | |||
const mod = require(path.join(fixtures.fixturesDir, 'global', 'plain')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am going to fix this while landing.
The commit message does not follow the commit guidelines that are linked in each new PR description (it was actually removed from the PR description). |
PR-URL: #15840 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Landed in 8597a18 Thanks for the PR, and congratulations on becoming a Node.js Contributor 🎉 ! |
PR-URL: #15840 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs/node#15840 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs/node#15840 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
First-time contribution during Node.js Interactive:
In
test/parallel/test-global.js
, replacecommon.fixturesDir
with usage of thecommon.fixtures
module (see https://github.com/nodejs/node/tree/master/test/common#fixtures-module).