Skip to content

Add createConnection option for http or https requests#120

Merged
rexxars merged 1 commit into
EventSource:masterfrom
xv2:master
Jan 25, 2020
Merged

Add createConnection option for http or https requests#120
rexxars merged 1 commit into
EventSource:masterfrom
xv2:master

Conversation

@xv2

@xv2 xv2 commented Mar 13, 2019

Copy link
Copy Markdown
Contributor

issue #119

@thedillonb

Copy link
Copy Markdown

@aslakhellesoy any chance we can get this merged?

@rexxars rexxars merged commit 4ad734b into EventSource:master Jan 25, 2020
@rexxars

rexxars commented Jan 25, 2020

Copy link
Copy Markdown
Member

Thanks for contributing!

@aslakhellesoy

Copy link
Copy Markdown
Contributor

The supplied test won’t fail if createConnection isn’t called. The test just sets a boolean flag which is never checked.

The feature is undocumented. This didn’t seem quite ready to be merged. Sorry for my late review!

@rexxars

rexxars commented Jan 27, 2020

Copy link
Copy Markdown
Member

It's checked on line 611, in the request handler - assert.ok(testResult). done() is not called apart from within that handler. Am I not reading the test correctly?

I'll add documentation for the feature unless you want this reverted.

@aslakhellesoy

Copy link
Copy Markdown
Contributor

You're right @rexxars - that will teach me to stop doing code reviews on my phone :-)

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