Change in vdsm[master]: convert print statements to functions

fromani at redhat.com fromani at redhat.com
Fri Mar 27 11:22:53 UTC 2015


Francesco Romani has posted comments on this change.

Change subject: convert print statements to functions
......................................................................


Patch Set 3:

(15 comments)

initial review. Most (but not all) comments boil down to two points
- sometimes 'from __future__ ...' seems not needed, as it seems the only change in a file
- sometimes 'from __future__' is nicely put as first line after the copyright block and before all imports, sometimes it is the very first line of the file. Why the latter? I see no problems with the former, and I like it more.

https://gerrit.ovirt.org/#/c/37997/3/client/vdsClient.py
File client/vdsClient.py:

Line 16: #
Line 17: # Refer to the README and COPYING files for full details of the license
Line 18: #
Line 19: 
Line 20: from __future__ import print_function
I didn't expect this but on second thought, why not?
Line 21: import sys
Line 22: import ast
Line 23: import getopt
Line 24: import traceback


https://gerrit.ovirt.org/#/c/37997/3/lib/vdsm/vdscli.py
File lib/vdsm/vdscli.py:

Line 18: #
Line 19: # Refer to the README and COPYING files for full details of the license
Line 20: #
Line 21: 
Line 22: from __future__ import print_function
do we need this there or it is just for consistency?
Line 23: import xmlrpclib
Line 24: import os
Line 25: import re
Line 26: import sys


https://gerrit.ovirt.org/#/c/37997/3/tests/cPopenTests.py
File tests/cPopenTests.py:

Line 1: from __future__ import print_function
why up here?
Line 2: #
Line 3: # Copyright 2012 Red Hat, Inc.
Line 4: #
Line 5: # This program is free software; you can redistribute it and/or modify


https://gerrit.ovirt.org/#/c/37997/3/tests/fileSDTests.py
File tests/fileSDTests.py:

Line 1: from __future__ import print_function
why up here?
Line 2: #
Line 3: # Copyright 2014 Red Hat, Inc.
Line 4: #
Line 5: # This program is free software; you can redistribute it and/or modify


https://gerrit.ovirt.org/#/c/37997/3/tests/miscTests.py
File tests/miscTests.py:

Line 1: from __future__ import print_function
why up here?
Line 2: #
Line 3: # Copyright 2012 Red Hat, Inc.
Line 4: #
Line 5: # This program is free software; you can redistribute it and/or modify


https://gerrit.ovirt.org/#/c/37997/3/tests/scheduleTests.py
File tests/scheduleTests.py:

Line 1: from __future__ import print_function
same, why up here?
Line 2: #
Line 3: # Copyright 2014 Red Hat, Inc.
Line 4: #
Line 5: # This program is free software; you can redistribute it and/or modify


https://gerrit.ovirt.org/#/c/37997/3/tests/sslTests.py
File tests/sslTests.py:

Line 17: #
Line 18: # Refer to the README and COPYING files for full details of the license
Line 19: #
Line 20: 
Line 21: from __future__ import print_function
do we need this or it is just for consistency?
Line 22: import errno
Line 23: import os
Line 24: import re
Line 25: import SimpleXMLRPCServer


https://gerrit.ovirt.org/#/c/37997/3/vdsm/hooks.py
File vdsm/hooks.py:

Line 1: from __future__ import print_function
same, why up here?
Line 2: #
Line 3: # Copyright 2010-2011 Red Hat, Inc.
Line 4: #
Line 5: # This program is free software; you can redistribute it and/or modify


https://gerrit.ovirt.org/#/c/37997/3/vdsm/network/api.py
File vdsm/network/api.py:

Line 16: #
Line 17: # Refer to the README and COPYING files for full details of the license
Line 18: #
Line 19: 
Line 20: from __future__ import print_function
do we need this or it is just for consistency?
Line 21: from functools import wraps
Line 22: import errno
Line 23: import inspect
Line 24: import sys


https://gerrit.ovirt.org/#/c/37997/3/vdsm/storage/threadPool.py
File vdsm/storage/threadPool.py:

Line 1: from __future__ import print_function
why up here?
Line 2: #
Line 3: # Adapted by Red Hat from
Line 4: # http://code.activestate.com/recipes/203871-a-generic-programming-thread-pool/
Line 5: # Author: Tim Lesher


https://gerrit.ovirt.org/#/c/37997/3/vdsm_hooks/allocate_net/before_device_create.py
File vdsm_hooks/allocate_net/before_device_create.py:

Line 17: # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
Line 18: #
Line 19: # Refer to the README and COPYING files for full details of the license
Line 20: #
Line 21: from __future__ import print_function
do we need this or it is for consistency's sake?
Line 22: import os
Line 23: import random
Line 24: 
Line 25: from xml.dom import minidom


https://gerrit.ovirt.org/#/c/37997/3/vdsm_hooks/ethtool_options/ethtool_options.py
File vdsm_hooks/ethtool_options/ethtool_options.py:

Line 62:     print('splits into: ')
Line 63:     for subcmd in _parse_into_subcommands(opts['ethtool_opts'].split()):
Line 64:         command = ([ETHTOOL_BINARY.cmd] + [subcmd.name, subcmd.device] +
Line 65:                    subcmd.flags)
Line 66:         print(('    '), end=' ')
why the 'end' parameter? Can't see the benefit, can you please elaborate?
Line 67:         print(command)
Line 68: 
Line 69:     # Test with the correct nics
Line 70:     nics = ('em1', 'em2')


https://gerrit.ovirt.org/#/c/37997/3/vdsm_hooks/openstacknet/openstacknet_utils.py
File vdsm_hooks/openstacknet/openstacknet_utils.py:

Line 1: #!/usr/bin/python
Line 2: 
Line 3: from __future__ import print_function
do we need this or it is just for consistency?
Line 4: import hooking
Line 5: import subprocess
Line 6: from vdsm.netinfo import DUMMY_BRIDGE
Line 7: from vdsm.utils import CommandPath


https://gerrit.ovirt.org/#/c/37997/3/vdsm_hooks/vmfex_dev/vmfex_vnic.py
File vdsm_hooks/vmfex_dev/vmfex_vnic.py:

Line 24: different events:
Line 25:     * before_device_create
Line 26:     * before_nic_hotplug
Line 27: """
Line 28: from __future__ import print_function
do we need this or it is for consistency?
Line 29: from xml.dom import minidom
Line 30: import fcntl
Line 31: import os
Line 32: import sys


https://gerrit.ovirt.org/#/c/37997/3/vdsm_reg/deployUtil.py.in
File vdsm_reg/deployUtil.py.in:

Line 1: from __future__ import print_function
why up here?
Line 2: #
Line 3: # Copyright 2009-2012 Red Hat, Inc.
Line 4: #
Line 5: # This program is free software; you can redistribute it and/or modify


-- 
To view, visit https://gerrit.ovirt.org/37997
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I75222d243f27b557ea4b87522b9cf2113534e376
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuznets at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list