On Wed, Dec 19, 2012 at 01:43:34AM +0200, Zeeshan Ali (Khattak) wrote: > On Wed, Dec 19, 2012 at 12:33 AM, Christophe Fergeau > <cfergeau@xxxxxxxxxx> wrote: > > You are the one saying we should deprecate some methods, and at the same > > time saying we should not be deprecating things. > > I've really had it with you generalizing my statements and taking them > out of context. Well, this was a bit tongue in cheek, but what I meant is I'm not pushing to deprecate stuff. On the other hand, in this very mail, you are saying: « I don't think we should be deprecating APIs without a good reason. » and « Its like deprecating API silently. » > > > As far as I'm concerned, after applying the whole patch series, the old API > > is still working as always, I don't see reasons for it being broken any > > time soon, and as far as I'm concerned the old API can stay. > > As I said twice already, that will mean confusing the app developer > with two competing APIs. 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. > > > 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, ... then it's more convenient to get the OsinfoInstallConfigParamList from the config as you may not have as script handy. > > I'd just like that we export osinfo_install_config_new_for_script and > > promote its use in our docs as imo this will be useful moving forward, but > > that's it. > > Not deprecating existing API (now) while adding a competing API to it > and promoting that in the docs is no solution. Its like deprecating > API silently. It's not a competing API, it's just convenience API that will ensure that OsinfoInstallConfig:config-params is set on your newly created OsinfoInstallConfig object. You can also set it by hand, or not set it at all if you don't need it. What I'm suggesting complements and improves the existing API, it does not really compete with it. > Once again, I'll propose that Daniel makes the decision here. I don't > know why you keep ignoring that suggestion. I'm not ignoring it. I'm still refining my thoughts on this subject, and slightly changing my point of view on the approach we should take in the course of this email thread, so I'm echoing this here. Daniel's input on that topic is obviously very welcome, but if we reach an agreement without him, that's good as well. To summarize my current take on this, both OsinfoInstallConfig:config-params and OsinfoInstallScript:config-params are good to have. Depending on the use case, both can be useful, so we should keep both. We add two new methods osinfo_install_config_new_for_script() and osinfo_install_config_get_config_params(). We don't deprecate anything. osinfo_install_script_get_config_params() and osinfo_install_config_new() will still be there as they are useful in different context. And I don't see any huge confusion arising from that that cannot be fixed by some basic documentation (I said 'basic', there is no complicated confusing magic to understand there). Christophe
Attachment:
pgpkNvsqpVJAS.pgp
Description: PGP signature
_______________________________________________ Libosinfo mailing list Libosinfo@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libosinfo
