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
[web]: Simplifies circuits.web.HTTP properties by removing useless hasattr/getattr checks #211
Conversation
return "https" if self._server.secure else "http" | ||
|
||
@property | ||
def base(self): | ||
if getattr(self, "uri", None) is None: |
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.
If self.uri
is None
(ie before the ready
handler ran), the line below self.uri.utf8()
would error out if. I guess you still need to check if self.uri
is None
but you can remove the getattr
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.
Good catch. Fixed.
5eaedf0
to
df4d609
Compare
Current coverage is 77.89% (diff: 50.00%)@@ master #211 diff @@
==========================================
Files 80 80
Lines 6641 6637 -4
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
- Hits 5175 5170 -5
- Misses 1466 1467 +1
Partials 0 0
|
Review please :) |
if getattr(self, "uri", None) is None: | ||
return | ||
if self.uri is None: | ||
return "" |
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.
Wrong... return b""!
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.
Then the code below is also wrong since it explicitelly decodes ;)
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.
oh yes... then everything is fine.
No description provided.