On Tue, Jul 25, 2017 at 3:13 AM, Lukáš Doktor <ldok...@redhat.com> wrote: > Dne 25.7.2017 v 08:04 Philippe Mathieu-Daudé napsal(a): >> Hi Lukáš, >> >> On 07/24/2017 09:36 AM, Lukáš Doktor wrote: >>> Dne 22.7.2017 v 03:30 Philippe Mathieu-Daudé napsal(a): >>>> Hi Lukáš, >>>> >>>> Since comment/indent fixes and code changes are not related I'd rather see >>>> this split in at least 2 patches. >>>> >>> Hello Philippe, thank you for the review, I'm wondering what code changes >>> you have in mind? This is commit should not bring any code changes, just >>> code reorganization (like including the self.* attributes on top of the >>> file) >>> >>>> On 07/20/2017 01:28 PM, Lukáš Doktor wrote: >>>>> No actual code changes, just a few pylint/style fixes and docstring >>>>> clarifications. >>>>> >>>>> Signed-off-by: Lukáš Doktor <ldok...@redhat.com> >>>>> --- >>>>> scripts/qmp/qmp.py | 37 ++++++++++++++++++++++++------------- >>>>> 1 file changed, 24 insertions(+), 13 deletions(-) >>>>> >>>>> diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py >> [...] >>>>> def __init__(self, address, server=False, debug=False): >>>>> """ >>>>> Create a QEMUMonitorProtocol class. >>>>> @@ -42,6 +53,7 @@ class QEMUMonitorProtocol: >>>>> self.__address = address >>>>> self._debug = debug >>>>> self.__sock = self.__get_sock() >>>>> + self.__sockfile = None >> >> I was thinking about this line which is new. Now the declaration and >> initialization are done in __init__() while before it was only >> declared/initialized in connect() or accept(). >> >> I'm not expert of python interpreter/jit internals but expect the generated >> code to be slightly different, even if achieving the same. >> >> not a bit deal, just about wording ;) >> > Well the difference is that before `connect` you get `AttributeError` when > looking for `self.__sockfile` while with this patch you'll get `None`. In > this code nobody queries for `self.__sockfile` before the `connect` so it > should not change the behavior of this code so I consider that as a `style` > fix as it's ugly to extend attributes later in code (with some exceptions > like Namespace-objects..). Anyway if you insist I can split those patches.
I'm not insisting ;) Just add something like "also initialize __sockfile to avoid AttributeError while introspecting object before any call to connect/accept" in the commit message and that's fine to me.