On 05/30/2018 05:31 PM, Eduardo Habkost wrote:
> On Wed, May 30, 2018 at 05:03:56PM -0400, Cleber Rosa wrote:
>> On 05/30/2018 04:00 PM, Eduardo Habkost wrote:
> [...]
>> [...] Now, in addition to this very personal, and thus
>> insignificant botheration, it restricts API design. By signaling to
>> users that this is valid:
>>
>> self.vm.add_args(...).launch(...).shutdown()
>>
>> We'd be restricted on the design of, say, migrate(). Even if it sounds
>> sensible to return a boolean indicating migration success, for
>> consistency, it'd require a "return self".
> [...]
>>> I don't see why this would be an argument against making this
>>> possible:
>>>
>>> self.vm.add_args('-nodefaults', '-S').launch()
>>
>> Right, it's *mostly* style. But, it suggests that all methods work
>> similarly and thus restricts API design as mentioned before.
> [...]
>>> They are more readable than the complex method chaining examples
>>> you have shown, but I don't see any argument against:
>>>
>>> self.vm.add_args(...).launch()
>>
>> Right. This one looks nice indeed. *My* issues are related to
>> expectations (all methods support this?), consistency (all methods
>> should support this!) and the design limitations it brings.
>>
>
> These are good points.
>
> I believe it would be a reasonable convention to make methods
> that change QEMU configuration return self (because they are
> often called one after another), but other methods like launch()
> migrate(), and shutdown() don't have to.
>
> So this would work:
>
> self.vm.add_args(...).add_drive(...).launch()
>
> But this doesn't have to:
>
> self.vm.add_args(...).launch().migrate().shutdown()
>
Looks like a sane general rule. Let's revisit this when new add_*()
methods are added.
- Cleber.
