◐ Shell
reader mode source ↗
Skip to content

test: replace common.fixturesDir with common.fixtures#15895

Closed
tkg214 wants to merge 1 commit into
nodejs:masterfrom
tkg214:patch-test-first-commit
Closed

test: replace common.fixturesDir with common.fixtures#15895
tkg214 wants to merge 1 commit into
nodejs:masterfrom
tkg214:patch-test-first-commit

Conversation

@tkg214

@tkg214 tkg214 commented Oct 6, 2017 •
edited
Loading

Copy link
Copy Markdown
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Oct 6, 2017
@mscdex mscdex added the https Issues or PRs related to the https subsystem. label Oct 6, 2017
@Trott Trott added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Oct 6, 2017
BridgeAR
BridgeAR previously requested changes Oct 9, 2017
View reviewed changes

@BridgeAR BridgeAR left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

Just marking this so it does not get landed until this is fixed.

@BridgeAR

BridgeAR commented Oct 18, 2017

Copy link
Copy Markdown
Member

Ping @tkg214

@apapirovski

apapirovski commented Oct 25, 2017

Copy link
Copy Markdown
Contributor

Hi @tkg214, would you like to follow up on the requested changes so this can land? Let me know if any clarifications are needed. Thanks for helping us improve Node!

@Trott Trott force-pushed the patch-test-first-commit branch from e55f865 to 40b8259 Compare October 28, 2017 02:04
@Trott

Trott commented Oct 28, 2017

Copy link
Copy Markdown
Member

I went ahead and fixed this up per @jasnell's suggestion (and the original intention of the task). Hope that's OK.

This should be good to land. PTAL @BridgeAR

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

joyeecheung
joyeecheung approved these changes Oct 28, 2017
View reviewed changes

@joyeecheung joyeecheung left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

LGTM, CI failures look unrelated

@gireeshpunathil

gireeshpunathil commented Oct 31, 2017

Copy link
Copy Markdown
Member

ping @BridgeAR again to have another look ( one of 3 remaining code-and-learn PRs!)

@Trott Trott dismissed BridgeAR’s stale review November 1, 2017 03:53

Requested changes have been made.

Trott pushed a commit to Trott/io.js that referenced this pull request Nov 1, 2017
@t-k-g @Trott
test: use fixtures module in test-https-pfx …
c52fe67 
PR-URL: nodejs#15895
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Trott

Trott commented Nov 1, 2017

Copy link
Copy Markdown
Member

Landed in c52fe67.

Thanks for the contribution! 🎉

@Trott Trott closed this Nov 1, 2017
Qard pushed a commit to ayojs/ayo that referenced this pull request Nov 2, 2017
@t-k-g @Qard
test: use fixtures module in test-https-pfx …
83a0540 
PR-URL: nodejs/node#15895
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Qard pushed a commit to ayojs/ayo that referenced this pull request Nov 2, 2017
@t-k-g @Qard
test: use fixtures module in test-https-pfx …
1f19d4d 
PR-URL: nodejs/node#15895
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
cjihrig pushed a commit to cjihrig/node that referenced this pull request Nov 6, 2017
@t-k-g @cjihrig
test: use fixtures module in test-https-pfx …
f9b2099 
PR-URL: nodejs#15895
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@cjihrig cjihrig mentioned this pull request Nov 6, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Nov 14, 2017
@t-k-g @MylesBorins
test: use fixtures module in test-https-pfx …
9416dab 
PR-URL: #15895
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
gibfahn pushed a commit that referenced this pull request Nov 14, 2017
@t-k-g @gibfahn
test: use fixtures module in test-https-pfx …
2f66faf 
PR-URL: #15895
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Nov 21, 2017
Merged
@gibfahn gibfahn mentioned this pull request Nov 21, 2017
Merged
addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017
@t-k-g @addaleax
test: use fixtures module in test-https-pfx …
f8f2ebc 
PR-URL: nodejs/node#15895
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell left review comments

@watilde watilde watilde approved these changes

@cjihrig cjihrig cjihrig approved these changes

@joyeecheung joyeecheung joyeecheung approved these changes

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

@BridgeAR BridgeAR BridgeAR left review comments

Assignees

No one assigned

Labels

code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. https Issues or PRs related to the https subsystem. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.