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.

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

Support options that is classes with props #1886

Closed
wants to merge 1 commit into from

Conversation

mogadanez
Copy link

@brendanashworth
Copy link
Contributor

ref: nodejs/node-v0.x-archive#25454

Could you make the commit log conform to this please? Thanks!

@brendanashworth brendanashworth added the http Issues or PRs related to the http subsystem. label Jun 3, 2015
@mscdex
Copy link
Contributor

mscdex commented Jun 3, 2015

If we make this change here, should we be changing it everywhere else where we do something similar to maintain consistency?

@brendanashworth
Copy link
Contributor

If we make this change here, should we be changing it everywhere else where we do something similar to maintain consistency?

@jonathanong did some with #635, so I guess so, where it's necessary. I guess someone will want it at some point in time.

Oh - and this also needs a regression test 😄

@jonathanong
Copy link
Contributor

gotta make sure it's not gonna break npm :)

@brendanashworth
Copy link
Contributor

Running the test suite locally reveals that this has bad consequences for tests related to https, I count 17 non-flaky failures.

@vkurchatkin
Copy link
Contributor

@brendanashworth That's because of hasOwnProperty checks probably. We need to get rid of those before

@brendanashworth
Copy link
Contributor

@mogadanez are you able to make the requested changes?

@brendanashworth
Copy link
Contributor

I'm going to close this as some work still needs to be done on this, yet it is still stalled. If someone wants to pick it back up, they're more than welcome to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants