Fix 520 drop python2 #564
Fix 520 drop python2 #564
Conversation
g-k
force-pushed
the
fix-520-drop-python2
branch
from
5f97272
to
338e31d
Compare
g-k
force-pushed
the
fix-520-drop-python2
branch
from
31aa0c0
to
bb6a31a
Compare
|
A 4.x milestone. That's phenomenal! I'll try to review for feedback this tomorrow. If there's anything you want me to pay particular attention to, let me know. |
| # x.y.z or x.y.z.dev0 -- semver | ||
| __version__ = "3.2.1" | ||
| __version__ = "4.0.0.dev0" |
There was a problem hiding this comment.
should we release an alpha/beta/rc first?
There was a problem hiding this comment.
I think the nicety of an alpha/beta/rc comes only when you've got people lined up to use/test it that are representative of Bleach users. If you don't, then you spend a bunch of time doing the alpha/beta/rc, wait a bunch of time, and then it's not clear whether that extra effort bought you anything or reduced the chances of a follow-up fixit release.
In the past, I've kept tabs on bugs and done follow-up fixit releases. I can usually do that pretty quickly.
If it were me, I'd release on a Monday with a few following days that I could turn around a fixit release and not go the alpha/beta/rc route.
| raise TypeError("argument must be of text type") | ||
|
|
||
| text = force_unicode(text) |
There was a problem hiding this comment.
force_unicode checks text it coerces is valid utf-8, but the encoding arg for str uses sys.getdefaultencoding so maybe we should validate that.
There was a problem hiding this comment.
I don't think we need to worry about this. If the text value isn't a str, then we don't need to decode it so we don't need to validate encodings.
I think what you did is right and all we need to do is remove the force_unicode call.
|
Awesome! No rush on the taking a look. There's a lot going on. Left a couple comments on things I wasn't sure of. I don't think I did anything crazy (mainly swapping |
| @@ -45,7 +45,7 @@ def get_version(): | |||
| include_package_data=True, | |||
| package_data={'': ['README.rst']}, | |||
| zip_safe=False, | |||
| python_requires='>=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*', | |||
|
This looked fine to me. I didn't look at the code that's not covered in this PR, but it's probably fine and if there are other things we could clean up post-dropping-Python-2, we can do them later. |
Fix #520
Changes:
setup.pymetadata andCHANGESsixin non-vendored codeThis doesn't:
sixsince html5lib needs itI'm planning to squash everything down into 1 to 3 commits if it looks OK.
f? @willkg