Skip to content

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

Merged
wRAR merged 1 commit intoscrapy:masterfrom
sbs2001:patch-10
Nov 19, 2019
Merged

Update reactor.py, updated 'if' sequencing .#3937
wRAR merged 1 commit intoscrapy:masterfrom
sbs2001:patch-10

Conversation

@sbs2001
Copy link
Contributor

@sbs2001 sbs2001 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
Copy link

codecov bot 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
Copy link
Member

@kmike kmike left a comment

Choose a reason for hiding this comment

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

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

@wRAR wRAR merged commit e829c47 into scrapy:master Nov 19, 2019
Webson35

This comment was marked as spam.

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.

4 participants