Page MenuHomePhabricator

Python 2/3 compat - socket server
ClosedPublic

Authored by serge-sans-paille on Dec 4 2018, 2:19 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

Is there a particular reason why this and https://reviews.llvm.org/D55259 are separate patches?

Is there a particular reason why this and https://reviews.llvm.org/D55259 are separate patches?

The Python 2/3 portability is a huge patch. I have split it in several micro-patch, one for each package/feature impacted. The underlying idea is that we can keep the discussions focus. This package should just be a renaming issue, while https://reviews.llvm.org/D55259 uses a higher level API than the one before.

gbedwell accepted this revision.Dec 12 2018, 6:27 AM
gbedwell added a subscriber: gbedwell.

LGTM with one minor nit.

tools/scan-view/bin/scan-view
90 ↗(On Diff #176572)

Minor nit. Elsewhere (E.g. D55202 and D55200), the pattern:

try:
    import py3name
except ImportError:
    import py2name as py3name

is used, so for consistency I'd suggest doing the same here (it also has the nice property of implying that Python 3 should be the 'real' version).

This revision is now accepted and ready to land.Dec 12 2018, 6:27 AM
Closed by commit rL349010: Portable Python script across Python version (authored by serge_sans_paille, committed by ). · Explain WhyDec 12 2018, 11:48 PM
This revision was automatically updated to reflect the committed changes.

Thanks @gbedwell for the review o/