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

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

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

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.


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