[Workman-devel] [PATCH] clean up the API

[Josh Poimboeuf]

On 02/22/2013 06:28 AM, Daniel P. Berrange wrote:
> On Wed, Feb 20, 2013 at 02:43:43PM -0600, Josh Poimboeuf wrote:
>> Because partitions can be nested within other partitions, and because
>> partitions can be persistent and/or active, move the following methods
>> from WorkmanConsumer to WorkmanObject:
>> - workman_consumer_set_partition
>> - workman_consumer_get_partition
>> - workman_consumer_get_state (formerly get_persistent and get_active)
>
> I don't like this change because is presumes that the only subclasses
> of WorkmanObject are those which are related to things in partitions.
> In the future I anticipate us having more object types where these
> methods won't make sense.

Ok, makes sense for partitions.

For states, might it be possible for the future objects to be persistent
and/or active?  We may end up wanting to persist the non-cgroup-related
/proc and /sysfs tunables across reboots.  And even if we don't, the
state concept remains and they would just always be "active".

> So if you really want to have these methods
> in a parent class, then we'll need to have some new intermediate
> parent class between WorkmanObject &  WorkmanConsumer /WorkmanPartition
> that holds them.

Ok.  I'm on the fence about this one, but I think I'll skip the
intermediate class for now, as I'm allergic to deep levels of
inheritance.

>
>> Change the order of the arguments of workman_manager_add_partition to be
>> more consistent with the rest of the API, for which the state is usually
>> the 2nd argument.
>>
>> Add the partition argument to workman_manager_remove_partition.
>>
>> Rename workman_partition_get_subdivisions to
>> workman_partition_get_subpartitions, which is clearer.
>>
>> Add state argument to workman_partition_get_consumers and
>> workman_partition_get_subpartitions, because a partition's children can
>> be associated with active and/or persistent states.
>
> These make sense, but each of these changes really ought to be
> separate commits, since they are not dependant on each other.

Fair enough.


Josh