-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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: modify common.fixturesDir to fixtures module #16095
Conversation
hello! I'm new to contributing to node. Could I get some guidance on how to make my PR not have commit 75318e4 included in the changes? Thank you so much :) |
Hi, you can do this by rebasing your working branch with upstream/master: $ git rebase -i upstream/master Then delete the the commit by removing this line:
Then you will be left with only your single commit and you can push your changes to your branch (will require you to force push using git push -f origin code-learn). Let me know if the above does not make sense and I'll be happy to help out. |
@emyl3 Hi, can you rebase this PR by trying the suggestions in #16095 (comment) ? |
Hi! Thank you for your help. I rebased and then successfully removed the line you mentioned from my commit message but when I force pushed from my local elisa/code-learn branch it said everything was up to date. Please let me know what else I should try! Thank you again :) |
const assert = require('assert'); | ||
const path = require('path'); | ||
|
||
const dir = path.join(common.fixturesDir, 'GH-7131'); | ||
const dir = path.join(fixtures.fixturesDir, 'GH-7131'); |
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.
Please use fixtures.path
instead.
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.
Specifically, this would be:
const dir = fixtures.path('GH-7131');
@emyl3 how did you rebase? And did you fetch from upstream? |
9fd0614
to
db933e3
Compare
@BridgeAR I think I fixed it? Please let me know if this works! Thank you for helping me out! |
CI: https://ci.nodejs.org/job/node-test-pull-request/10841/ |
@@ -1,10 +1,12 @@ | |||
// Flags: --no-deprecation | |||
|
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.
Nit: this change seems unnecessary. Can be removed during landing
Landed in ca12ae6, thank you for your contribution! 🎉 |
PR-URL: #16095 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #16095 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs/node#16095 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs/node#16095 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)