Skip to content
This repository has been archived by the owner on May 9, 2020. It is now read-only.

Release 3.0.0 #106

Merged
merged 38 commits into from
Mar 8, 2019
Merged

Release 3.0.0 #106

merged 38 commits into from
Mar 8, 2019

Conversation

codemanki
Copy link
Owner

@codemanki codemanki commented Mar 4, 2019

@pro-src Let's use this PR for further discussions, also i will prepare readme and docs

  • Rewrite README to reflect new changes
  • Add upgrade guide for 2.0 -> 3.0

@ghost
Copy link

ghost commented Mar 5, 2019

@codemanki that ✔️ looks great! I'm super glad that we have these checks in place!

@codemanki
Copy link
Owner Author

@pro-src can we do a code freeze on this PR? So we that anything else goes into 3.0.0?

@ghost
Copy link

ghost commented Mar 7, 2019

@codemanki, of course.

@@ -26,6 +29,22 @@ __Unfortunately, there is no support for handling a CAPTCHA, if the response con

If you notice that for some reason cloudscraper stopped to work, do not hesitate and get in touch with me ( by creating an issue here, for example), so i can update it.

Migration from v2 to v3
Copy link
Owner Author

Choose a reason for hiding this comment

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

@pro-src I hope I didn't miss any other crucial points?

Copy link

Choose a reason for hiding this comment

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

@codemanki LGTM

Copy link

Choose a reason for hiding this comment

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

We might want to advice proper usage of error.error which can be an instanceof Error in some cases. request-promise defines error.error but I believe it is a legacy attribute. I think we should promote error.cause since we're not setting those attributes ourselves. It's request/promise-core's error library that is being used to set those attributes.

Copy link

Choose a reason for hiding this comment

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

@codemanki, The Build Status badge at the very top of this README.md should be removed. It's a duplicate.

@codemanki
Copy link
Owner Author

Phew. I'm done with README. @pro-src please take a look and let me know if you approve this PR and I can release it as 3.0.0

README.md Outdated
@@ -180,7 +180,7 @@ Let me know, by opening [issue](https://github.com/codemanki/cloudscraper/issues

WAT
===========
Current cloudflare implementation requires browser to respect the timeout of 5 seconds and cloudscraper mimics this behaviour. So everytime you call `cloudscraper.get/post` you should expect it to return result after minimum 6 seconds. If you want to change this behaviour, you would need to make a generic request as desceribed in above and pass `cloudflareTimeout` options with your value. But be aware that cloudflare might track this timeout and use ir against you ;)
Current cloudflare implementation requires browser to respect the timeout of 5 seconds and cloudscraper mimics this behaviour. So everytime you call `cloudscraper.get/post` you should expect it to return result after minimum 6 seconds. If you want to change this behaviour, you would need to make a generic request as desceribed in above and pass `cloudflareTimeout` options with your value. But be aware that cloudflare might track this timeout and use it against you ;)
Copy link

Choose a reason for hiding this comment

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

You missed one desceribed 😄

Copy link
Owner Author

Choose a reason for hiding this comment

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

Nooooooooooo

Copy link

Choose a reason for hiding this comment

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

Another one in the the TODO: chalenge should be challenge 🤣

Copy link
Owner Author

Choose a reason for hiding this comment

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

That is a legacy typo, been there for a long time. But thank you :)

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

@codemanki I'm going to look over everything thoroughly and I'll get back to you by at least tomorrow.

@codemanki
Copy link
Owner Author

@pro-src great :) looking forward to hearing from you. Thanks!

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

The cloudscraper.defaults convenience method isn't mentioned.

var cloudscraper = require('cloudscraper').defaults({ 'proxy': 'http://localproxy.com' });
// Override headers
var headers = { /* ... */ };
var cloudscraper = require('cloudscraper').defaults({ headers: headers });
// Access cloudscraper's default options. (It's not necessary to modify them)
// Create your own default options by using the defaults method.
console.log(cloudscraper.defaultParams);

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@@ -26,6 +29,22 @@ __Unfortunately, there is no support for handling a CAPTCHA, if the response con

If you notice that for some reason cloudscraper stopped to work, do not hesitate and get in touch with me ( by creating an issue here, for example), so i can update it.

Migration from v2 to v3
Copy link

Choose a reason for hiding this comment

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

@codemanki, The Build Status badge at the very top of this README.md should be removed. It's a duplicate.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Add short description to errors of type CloudflareError
@codemanki
Copy link
Owner Author

Off we go? @pro-src

@ghost
Copy link

ghost commented Mar 8, 2019

Indeed.

@codemanki codemanki merged commit c8c4100 into master Mar 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant