[py3porters-devel] Porting reportbug to Python 3

[Florian Bruhin]

* Thomas Goirand <zigo at debian.org> [2015-06-01 10:37:46 +0200]:
> Your comments are welcome, and as well, anyone from the py3porters team
> is more than welcome to review my patch.

I hope the ML is the right place for this - I did a quick look at the
patch and added some thoughts below.

Also, is it desirable to keep python2 compatibility?

> diff --git a/bin/reportbug b/bin/reportbug
> index 969e798..7ccdeab 100755
> --- a/bin/reportbug
> +++ b/bin/reportbug
> @@ -21,19 +21,34 @@
>  ##  ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS
>  ##  SOFTWARE.
>  
> +from __future__ import print_function
> +
>  DEFAULT_BTS = 'debian'
>  
>  import sys
>  
> -reload(sys)
> -sys.setdefaultencoding("utf-8")
> +try:
> +    reload(sys)
> +except:
> +    from imp import reload
> +    reload(sys)

You're catching other, unrelated exceptions here which might hide real
bugs.[1]

I'd recommend doing something like this:

    try:
        from imp import reload
    except ImportError:
        pass
    reload(sys)

[1] https://docs.python.org/2/howto/doanddont.html#except

> +try:
> +    sys.setdefaultencoding("utf-8")
> +except:
> +    print("")

Same issue as above - and what's the print("") for?

> -import commands
> -import rfc822
> +try:
> +    import commands
> +except:
> +    import subprocess
> +try:
> +    import rfc822
> +except:
> +    import email

"except ImportError:"

And you might want to do some "as"-imports and/or only import certain
classes so you end up getting the same classes to use for the
following code.

> diff --git a/reportbug/utils.py b/reportbug/utils.py
> index fede038..0708535 100644
> --- a/reportbug/utils.py
> +++ b/reportbug/utils.py
> @@ -20,26 +20,36 @@
> [...]
> -import commands
> +try:
> +    import commands
> +except:
> +    import subprocess
>  import shlex
> -import rfc822
> +try:
> +    import rfc822
> +except:
> +    import email
>  import socket
>  import subprocess
>  
> -from urlutils import open_url
> +try:
> +    from urlutils import open_url
> +except:
> +    from urllib.request import urlopen as open_url

Same thing as mentioned above.

> @@ -794,15 +804,15 @@ def get_cpu_cores():

FWIW, python 2.6+ has multiprocessing.cpu_count()[1]

[1] https://docs.python.org/3/library/multiprocessing.html#multiprocessing.cpu_count

> -	#Alpha platform
> -	if line.startswith('cpus detected'):
> -	    cpucount = int(line.split()[-1])
> +        #Alpha platform
> +        if line.startswith('cpus detected'):
> +            cpucount = int(line.split()[-1])

I don't have the context here - but did you intend to dedent this?



Thanks for your effort!

Florian

-- 
http://www.the-compiler.org | me at the-compiler.org (Mail/XMPP)
   GPG: 916E B0C8 FD55 A072 | http://the-compiler.org/pubkey.asc
         I love long mails! | http://email.is-not-s.ms/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/py3porters-devel/attachments/20150601/c8f25ad6/attachment.sig>