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.

Sign up for GitHub

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

Jump to bottom

Request: remove restriction about errback without callback #4008

Merged
merged 1 commit into from Jan 23, 2020
Merged

Request: remove restriction about errback without callback #4008

merged 1 commit into from Jan 23, 2020

Conversation

elacuesta
Copy link
Member

@elacuesta elacuesta commented Sep 11, 2019 •
edited

Fixes #3586

@codecov
Copy link

codecov bot commented Oct 3, 2019 •
edited

Codecov Report

Merging #4008 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4008      +/-   ##
==========================================
- Coverage   83.73%   83.73%   -0.01%     
==========================================
  Files         165      165              
  Lines        9714     9713       -1     
  Branches     1446     1446              
==========================================
- Hits         8134     8133       -1     
  Misses       1332     1332              
  Partials      248      248
Impacted Files Coverage Δ
scrapy/http/request/__init__.py 100% <ø> (ø) ⬆️

@elacuesta elacuesta changed the title [docs] Request: callback must be set explicitly when using errback Request: remove restriction about errback without callback Oct 3, 2019
@elacuesta elacuesta removed the docs label Oct 3, 2019
@elacuesta
Copy link
Member Author

elacuesta commented Oct 3, 2019 •
edited

Tests are green without the restriction (the one failure is because of #4014, it can be ignored IMHO).
@pablohoffman since you authored the commit that introduced the assertion, do you have any objections to remove it?

@pablohoffman
Copy link
Member

no objections @elacuesta!

@Gallaecio
Copy link
Member

Tests are green without the restriction

Unless we already have it, though, this might be a great opportunity to add a test for this, that specifically verifies that a callback can be omitted even if an errback is provided.

@kmike kmike merged commit f80c777 into scrapy:master Jan 23, 2020
@kmike
Copy link
Member

kmike commented Jan 23, 2020

Thanks @elacuesta!

@elacuesta elacuesta deleted the docs_request_errback branch January 23, 2020 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kmike kmike kmike left review comments

@Gallaecio Gallaecio Gallaecio approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Request doesn't set callback to self.parse automatically when errback is set
4 participants
@elacuesta @pablohoffman @Gallaecio @kmike