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

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



On Wed, Feb 20, 2013 at 02:43:43PM -0600, Josh Poimboeuf wrote:
> Replace workman_consumer_get_persistent and workman_consumer_get_active
> with workman_consumer_get_state.  This is more consistent with the rest
> of the API: using the WorkmanState enum rather than exposing its enum
> values in the interfaces.

Ok, makes sense.

> 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. 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.

> 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.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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