qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4 1/2] qemu.py: make 'args' public


From: Amador Pahim
Subject: Re: [Qemu-devel] [PATCH v4 1/2] qemu.py: make 'args' public
Date: Tue, 25 Jul 2017 16:41:37 +0200

On Tue, Jul 25, 2017 at 3:37 PM, Stefan Hajnoczi <address@hidden> wrote:
> On Mon, Jul 24, 2017 at 02:44:37PM +0200, Amador Pahim wrote:
>> Let's make args public so users can extend it without feeling like
>> abusing the internal API.
>
> Nothing is abusing an internal API.  PEP8 describes the difference
> between public (no underscore), protected aka subclass API (single
> underscore), and private fields (single or double underscore):
>
>   https://www.python.org/dev/peps/pep-0008/
>
> _args is a protected field.  It is perfectly normal for a subclass to
> access a protected field in its parent class.

Precisely. Idea is to make it public according to the concept of
public x protected from that very documentation ("Public attributes
are those that you expect unrelated clients of your class to use").
This makes sense for the work I'm doing on creating
functional/regression tests for qemu using Avocado Framework (patchs
to come), where we instantiate a QEMUMachine object to be offered
inside the Test API.

>
>> Signed-off-by: Amador Pahim <address@hidden>
>> Reviewed-by: Fam Zheng <address@hidden>
>> ---
>>  scripts/qemu.py               | 10 +++++-----
>>  tests/qemu-iotests/iotests.py | 18 +++++++++---------
>>  2 files changed, 14 insertions(+), 14 deletions(-)
>
> Please don't do this, it encourages code duplication.  Now arbitrary
> users can start accessing the public field directly instead of adding a
> reusable interfaces like add_monitor_telnet(), add_fd(), etc.

Reusable interfaces can be quite complex to implement when all you
need is to add some options to the qemu command line. I'd not require
that from arbitrary users. Anyway, dropping this commit. Thanks for
the review.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]