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

Update reactor.py, updated 'if' sequencing . #3937

Open
wants to merge 1 commit into
base: master
from

Conversation

@sbs2001
Copy link
Contributor

commented Aug 5, 2019

This should be the proper ordering.This is the explanation.
If 'not portrange' is True ,it is guaranteed that not hasattr(portrange, '__iter__') is also True the converse of this is not always true.(for example, consider portrange=None, for such case we were executing the logic for not hasattr(portrange, '__iter__') . ).Such case is eliminated by this PR.

…ug if portrange=None

This should be the proper ordering.This is the explanation.
  If 'not portrange' is True ,it is guaranteed that `not hasattr(portrange, '__iter__')`  is also True  the converse of this is not always true.(for example, consider portrange=None, for such case we were executing the logic for `not hasattr(portrange, '__iter__')` . ).Such case is eliminated by this PR.
@codecov

This comment has been minimized.

Copy link

commented Aug 5, 2019

Codecov Report

Merging #3937 into master will not change coverage.
The diff coverage is 0%.

@@           Coverage Diff           @@
##           master    #3937   +/-   ##
=======================================
  Coverage   85.54%   85.54%           
=======================================
  Files         166      166           
  Lines        9681     9681           
  Branches     1445     1445           
=======================================
  Hits         8282     8282           
  Misses       1146     1146           
  Partials      253      253
Impacted Files Coverage Δ
scrapy/utils/reactor.py 70% <0%> (ø) ⬆️
@sbs2001 sbs2001 changed the title Update reactor.py, updated 'if' sequencing , possibly eliminating a b… Update reactor.py, updated 'if' sequencing . Aug 5, 2019
@Gallaecio Gallaecio requested review from kmike, dangra and pablohoffman Aug 8, 2019
@kmike
kmike approved these changes Aug 8, 2019
Copy link
Member

left a comment

I don't mind it any way, but the change looks fine to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.