KeyFile is a, umm, key file parser. Nothing telepathy specific. So doesn't make that much sense in a telepathy library's public API. ManagerFile is telepathy specific, built on top of the former, but ConnectionManager, ProtocolInfo, Account provide a higher level and better aggregated with info from other sources view to the data which is read from the manager files anyway. So probably should make it internal as well. Need to study whether somebody actually uses these classes directly, and for what reason.
Branch at URL attempts to fix this. Note that I did not "unexported"(TP_QT_NO_EXPORT) the classes as doing it would break the tests. Should we build key-file/manager-file.cpp in the tests itself?
(In reply to comment #1) > Branch at URL attempts to fix this. > > Note that I did not "unexported"(TP_QT_NO_EXPORT) the classes as doing it would > break the tests. Should we build key-file/manager-file.cpp in the tests itself? Build as part of the test binaries, without putting to a library in between? Yeah, that'd work I believe. We could actually do the same for TestBackdoors and remove it from tp-qt4 sources completely (unless it links to non-exported, rather than merely protected, but exported, tp-qt4 internal symbols - whether it does that, I can't remember). You could also have a static library consisting of KeyFile, ManagerFile and TestBackdoors and link that into the relevant tests. But anyway, just not installing headers for them accomplishes the main goal - to remove them from the API and ABI we support. Even if we still had it in the library private ABI, like TestBackdoors. But please experiment with your idea to see if we could remove them from there too - this would lead to slightly less symbols in the lib, so faster symbol lookup in runtime linking.
(In reply to comment #2) > You could also have a static library consisting of KeyFile, ManagerFile and > TestBackdoors and link that into the relevant tests. Oh and of course don't install it, except if installing the tests.
(In reply to comment #3) > (In reply to comment #2) > > You could also have a static library consisting of KeyFile, ManagerFile and > > TestBackdoors and link that into the relevant tests. > > Oh and of course don't install it, except if installing the tests. Umm and not even then, because it's static so relevant parts in it will be carried in the test binaries themself.
(In reply to comment #2) > (In reply to comment #1) > > Note that I did not "unexported"(TP_QT_NO_EXPORT) the classes as doing it would > > break the tests. Should we build key-file/manager-file.cpp in the tests itself? > > Build as part of the test binaries, without putting to a library in between? > Yeah, that'd work I believe. We could actually do the same for TestBackdoors > and remove it from tp-qt4 sources completely (unless it links to non-exported, > rather than merely protected, but exported, tp-qt4 internal symbols - whether > it does that, I can't remember). > > You could also have a static library consisting of KeyFile, ManagerFile and > TestBackdoors and link that into the relevant tests. Branch updated adding a new internal static library (telepathy-qt-test-backdoors) that includes ManagerFile, KeyFile and TestBackdoors and is linked by the interested tests.
Yeah very good. I also checked with the KDE Telepathy guys - seems that they are not using these APIs. So away with it please. My stance is that anybody who's currently using them should be able to port their code to using higher level view of the same data in ConnectionManager and Account (where it is further multiplexed with info from live connections and profile files). If not, we have to add further API exposing the info in those places, not keep the low-level access which is needlessly cumbersome anyway.
Tnx for the review, changes merged upstream. It will be in tp-qt 0.9.0.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.