search

ladjs / superagent

Ajax for Node.js and browsers (JS HTTP client). Maintained for @forwardemail, @ladjs, @spamscanner, @breejs, @cabinjs, and @lassjs.
https://ladjs.github.io/superagent/
MIT License
16.58k stars 1.33k forks source link

Feature/fix lookup #1674

Closed yunnysunny closed 2 years ago

yunnysunny commented 2 years ago

It's the fix of the pull request https://github.com/visionmedia/superagent/pull/1595

I also found that there were some error after running make test, some related to the wrong configuration of eslint, and others related to testcases. I will make another pull request to fix this errors.

And I think it's important to enable the ci check before any merges to aviod bugs coming from new code.

niftylettuce commented 2 years ago

Are you sure this is all good to go now?

yunnysunny commented 2 years ago

Yes, it's ok. You can first merge the request of https://github.com/visionmedia/superagent/pull/1677 , and then merge this, you will see all testcases is ok.

titanism commented 2 years ago

v7.1.2 released https://github.com/visionmedia/superagent/releases/tag/v7.1.2

jmcollin78 commented 2 years ago

Hello @titanism , the release 7.1.2 seems to not exists yet. I'm waiting for the lookup feature and I was glad to see there was a fix for this. What about tag 7.1.2 ?

titanism commented 2 years ago

v7.1.2 was unpublished from npm and GitHub as it failed to pass all tests in CI across all Node versions.

Do we need to merge both #1716 and #1717 @yunnysunny? Also see https://github.com/visionmedia/superagent/pull/1717#issuecomment-1087643543 (ref https://github.com/visionmedia/superagent/pull/1677#issuecomment-1086778466)

yunnysunny commented 2 years ago

v7.1.2 was unpublished from npm and GitHub as it failed to pass all tests in CI across all Node versions.

Do we need to merge both #1716 and #1717 @yunnysunny? Also see #1717 (comment) (ref #1677 (comment))

Try to merge https://github.com/visionmedia/superagent/pull/1716 , and see if the error is missing.