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

fix to pass the test at node v6.x #723

Merged
merged 1 commit into from
Apr 30, 2016
Merged

fix to pass the test at node v6.x #723

merged 1 commit into from
Apr 30, 2016

Conversation

iyuq
Copy link
Contributor

@iyuq iyuq commented Apr 29, 2016

I reopened the request and squashed the 3 commits into one commit.

@iyuq iyuq changed the title fix to pass the teest at node v6.x fix to pass the test at node v6.x Apr 29, 2016
@juliangruber
Copy link
Contributor

can you explain why this change is necessary for node 6? (btw +1 from me, I've never liked the implicit globals available through mocha)

@iyuq
Copy link
Contributor Author

iyuq commented Apr 29, 2016

@juliangruber because node v6.x's querystring is create by Object.create(null), it doesn't have the prototype of Object.prototype, so if call ctx.query.should it would throw a error.

@iyuq
Copy link
Contributor Author

iyuq commented Apr 29, 2016

@juliangruber see The object returned by querystring.parse() no longer inherits from Object.prototype #6055.

@juliangruber juliangruber merged commit c979056 into koajs:v2.x Apr 30, 2016
@juliangruber
Copy link
Contributor

thank you!

@iyuq
Copy link
Contributor Author

iyuq commented May 1, 2016

@juliangruber you're welcome! but why haven't it added to the master branch? I have also opened a pull request to the master branch.

@PlasmaPower
Copy link
Contributor

@iyuq I'm pretty sure that PR got closed as a duplicate of this one by accident :P it shouldn't be though, they are different branches.

@iyuq
Copy link
Contributor Author

iyuq commented May 1, 2016

@juliangruber so I should reopen that PR again?

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.

3 participants