[Libosinfo] [PATCHv4 06/11] Add OsinfoInstallConfig:config-params property

[Christophe Fergeau]

On Wed, Dec 19, 2012 at 04:55:23PM +0200, Zeeshan Ali (Khattak) wrote:
> On Wed, Dec 19, 2012 at 11:16 AM, Christophe Fergeau
> <cfergeau at redhat.com> wrote:
> > When I started working on these patches, I was very confused by
> > OsinfoInstallConfig VS OsinfoInstallConfigParam. They have similar names,
> > but they have no relationship whatsoever, you cannot get one from the
> > other, you are not using them in the same API calls, ... so the API is
> > confusing anyway.
> 
> I understand and agree that this API is confusing but by keeping it
> and adding the same API on another class will cause a lot more
> confusion. That would be actually contrary to what you are trying to
> achieve here.

I don't know what you are talking about here, use specific method and class
names and make a detailed answer. We don't seem to be talking about the
same thing.

> 
> >> > After looking at what Boxes does, it seems to make sense to have the
> >> > OsinfoInstallConfigParamList be available from both
> >> > OsinfoInstallScript and OsinfoInstallConfig depending on what is more
> >> > convenient for the user at a given time.
> >>
> >> How would one be more convenient than the other?
> >
> > If you have an OsinfoInstallScript object because you are setting up
> > 'stuff' which you'll then use to create OsinfoInstallConfig(s), then it's
> > more convenient to get the OsinfoInstallConfigParamList from the script as
> > you haven't started creating your config. This is the way Boxes is using
> > it.
> >
> > If you have already started your OsinfoInstallConfig and need to know which
> > parameters are mandatory, which are supported, ...
> 
> That is dependent on the script and config alone can not tell you
> that. The only way config can tell you this is to get this information
> from a script.
> 
> > then it's more
> > convenient to get the OsinfoInstallConfigParamList from the config as you
> > may not have as script handy.
> 
> You can't have this information without a script instance at hand for
> the reason I mentioned above. So I'm sorry but I don't see any
> convenience being added.

This avoids having to carry around an OsinfoInstallScript instance when all
you are doing is filling your OsinfoInstallConfig object, you don't have to
pass it through every method calls, ...
Had 'path-format' described as an OsinfoInstallConfigParam (which seems to
make sense from a quick glance), Boxes would even have been able to benefit
from that in UnattendedInstaller::configure_install_script, the
OsinfoInstallScript argument would not have been needed.


> > What I'm suggesting complements and improves the
> > existing API, it does not really compete with it.
> 
> So I take it that you have changed your opinion slightly on this cause
> you were the first to call these two approaches "competing" in this
> thread?

Well, I don't know what I used to be saying, but yeah I've said several
times in this email that my thinking on all of this was still evolving.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libosinfo/attachments/20121219/2a15a17d/attachment.sig>