Type annotations #172
Type annotations #172
Conversation
Codecov Report
@@ Coverage Diff @@
## master #172 +/- ##
==========================================
+ Coverage 94.70% 95.68% +0.98%
==========================================
Files 6 7 +1
Lines 434 464 +30
Branches 88 88
==========================================
+ Hits 411 444 +33
+ Misses 12 10 -2
+ Partials 11 10 -1
|
lucywang000
force-pushed
the
add-type-annotations
branch
2 times, most recently
from
c687b34 to
ba6f231
Compare
lucywang000
force-pushed
the
add-type-annotations
branch
2 times, most recently
from
1a89acb to
2af9b30
Compare
lucywang000
force-pushed
the
add-type-annotations
branch
2 times, most recently
from
16ec75f to
7c03de7
Compare
lucywang000
force-pushed
the
add-type-annotations
branch
from
7c03de7 to
1a81a47
Compare
|
@Gallaecio I think this patch is ready for a review :) |
| auth = auth.encode(encoding) | ||
| return b"Basic " + urlsafe_b64encode(auth) | ||
| auth = "%s:%s" % (to_native_str(username), to_native_str(password)) | ||
| # XXX: RFC 2617 doesn't define encoding, but ISO-8859-1 |
There was a problem hiding this comment.
is this comment still needed? I think it was about the encode call which is no longer done?
There was a problem hiding this comment.
I think it applies as long as we accept an encoding parameter here with that default value. We are still using the encoding, only now it has moved to the to_bytes call below.
| auth = auth.encode(encoding) | ||
| return b"Basic " + urlsafe_b64encode(auth) | ||
| auth = "%s:%s" % (to_native_str(username), to_native_str(password)) | ||
| # XXX: RFC 2617 doesn't define encoding, but ISO-8859-1 |
There was a problem hiding this comment.
I think it applies as long as we accept an encoding parameter here with that default value. We are still using the encoding, only now it has moved to the to_bytes call below.
Co-authored-by: Adrián Chaves <adrian@chaves.io>
|
@wRAR @Gallaecio Thanks for your review comments. I have addressed all your feedback. |
Improve ParseDataURIResult documentation
|
@Gallaecio I've merged your fix and made some changes to make the CI pass. |
|
@Gallaecio anything else I need to do to make this merged? |
|
I don’t think so. I’m not merging myself yet just in case @wRAR wants a final look first. |
|
Thanks guys! |
A new attempt after #123 .
The generated document now has type hints included as well: