[Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool

[Jan Cholasta]

Hi,

On 19.6.2013 09:48, Tomas Babej wrote:
> Hi,
>
> Provides a pluggable framework for generating configuration
> scriptlets and instructions for various machine setups.
>
> Creates a new ipa-client-advise command, available to root user
> on the IPA server.
>
> Also provides an example configuration plugin.

I don't like how you abuse our object model in this patch. For example, 
why does Configuration inherit from Method? It does not represent method 
of any object, it doesn't even represent a runnable command. I see you 
added an artificial advise object, which uses the ldap2 backend, but 
doesn't actually use LDAP, this is also ugly.

Please inherit from Plugin directly and create a new API namespace for 
advises instead. And don't call the class Configuration, it's misleading 
(Advise or Advisory is better IMHO).

Honza

-- 
Jan Cholasta