-
Notifications
You must be signed in to change notification settings - Fork 164
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
Some code simplifications with the return statement #877
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,10 +37,10 @@ def prepare_auth(auth, username, password): | |
if username and password: | ||
if auth == 'basic' or auth is None: | ||
return (username, password) | ||
elif auth == 'digest': | ||
if auth == 'digest': | ||
from requests.auth import HTTPDigestAuth | ||
return HTTPDigestAuth(username, password) | ||
elif auth == 'guess': | ||
if auth == 'guess': | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't really like this change. It makes it far harder to figure out the flow. At a glance, I'd get the impression that this code run The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In all programming languages, writing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It helps readability, IMO. |
||
try: | ||
from requests_toolbelt.auth.guess import GuessAuth | ||
except ImportError: | ||
|
@@ -57,8 +57,6 @@ def prepare_auth(auth, username, password): | |
elif auth: | ||
raise exceptions.UserError('You need to specify username and password ' | ||
'for {} authentication.'.format(auth)) | ||
else: | ||
return None | ||
|
||
|
||
def prepare_verify(verify, verify_fingerprint): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -122,7 +122,7 @@ def _parse_xml(content): | |
|
||
def _merge_xml(items): | ||
if not items: | ||
return None | ||
return | ||
rv = items[0] | ||
for item in items[1:]: | ||
rv.extend(item.iter()) | ||
|
@@ -138,9 +138,7 @@ def _fuzzy_matches_mimetype(strict, weak): | |
return True | ||
|
||
mediatype, subtype = strict.split('/') | ||
if subtype in weak: | ||
return True | ||
return False | ||
return subtype in weak | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a much welcome cleanup though. |
||
|
||
|
||
class Discover: | ||
|
@@ -236,7 +234,7 @@ def _check_collection_resource_type(self, response): | |
props = _merge_xml(response.findall( | ||
'{DAV:}propstat/{DAV:}prop' | ||
)) | ||
if props is None or not len(props): | ||
if props is None or not props: | ||
dav_logger.debug('Skipping, missing <prop>: %s', response) | ||
return False | ||
if props.find('{DAV:}resourcetype/' + self._resourcetype) \ | ||
|
@@ -587,7 +585,7 @@ def _parse_prop_responses(self, root, handled_hrefs=None): | |
continue | ||
|
||
props = response.findall('{DAV:}propstat/{DAV:}prop') | ||
if props is None or not len(props): | ||
if props is None or not props: | ||
dav_logger.debug('Skipping {!r}, properties are missing.' | ||
.format(href)) | ||
continue | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍