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.

Sign up for GitHub

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

Jump to bottom

replacing common with fixtures for cmd string building #15982

Closed
wants to merge 3 commits into from
Closed

replacing common with fixtures for cmd string building #15982

wants to merge 3 commits into from

Conversation

FNJ5
Copy link
Contributor

@FNJ5 FNJ5 commented Oct 6, 2017

Replaced the common module with the fixtures module for use when building the cmd string in test-error-reporting

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Oct 6, 2017
@mscdex mscdex added the errors Issues and PRs related to JavaScript errors originated in Node.js core. label Oct 6, 2017
@BridgeAR
Copy link
Member

BridgeAR commented Oct 7, 2017

The subsystem is missing and the commit message has to be fixed while landing

@Trott Trott added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Oct 7, 2017
@joyeecheung
Copy link
Member

CI: https://ci.nodejs.org/job/node-test-pull-request/10488/

@gibfahn
Copy link
Member

gibfahn commented Oct 8, 2017

Landed in b13e660

Thanks for the PR, and congratulations on becoming a Node.js Contributor 🎉 !

@gibfahn gibfahn closed this Oct 8, 2017
gibfahn pushed a commit that referenced this pull request Oct 8, 2017
@FNJ5 @gibfahn
test: use fixtures.path for cmd string building
b13e660 
PR-URL: #15982
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 11, 2017
@FNJ5 @MylesBorins
test: use fixtures.path for cmd string building
9592a48 
PR-URL: #15982
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Oct 11, 2017
Merged
addaleax pushed a commit to addaleax/ayo that referenced this pull request Oct 12, 2017
@FNJ5 @addaleax
test: use fixtures.path for cmd string building
177436b 
PR-URL: nodejs/node#15982
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gibfahn gibfahn gibfahn approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

@joyeecheung joyeecheung joyeecheung approved these changes

@tniessen tniessen tniessen approved these changes

@cjihrig cjihrig cjihrig approved these changes

@benjamingr benjamingr benjamingr approved these changes

Assignees
No one assigned
Labels
code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. errors Issues and PRs related to JavaScript errors originated in Node.js core. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.