Migration challenges from API changes in 6.0.0
I'm just writing here to document some of the challenges I've had as a downstream user of tinyxml2 due to the API changes in major version 6. I'm grateful for the work you do and don't want this to register as a complaint or a demand for fixes, but some ideas for making it easier for downstream users to migrate to new versions.
We have some code that attempts to print both GetErrorStr1 and GetErrorStr2 for diagnostic purposes. These functions were replaced by the single function ErrorStr in major version 6.
-
The old functions were removed without a deprecation tick-tock cycle, so instead of getting compiler warnings telling me that I should update my code, it fails to compile. I noticed this because homebrew recently updated to major version 6, so our code is broken for now on homebrew, while I work on a fix.
-
It's great that you define the API version numbers in your header file, but they are
static const intvariables, which means that I can't evaluate them in a compiler directive like#if TIXML2_MAJOR_VERSION >= 6, since only macros can be evaluated in compiler directives (I just learned this actually that for compiler directives Identifiers that are not macros ... are all considered to be the number zero). I believe I can work around this with some cmake code to detect the version and define a symbol if the version is 6 or greater, but it's a little more of a hassle.
Again, thanks for your work maintaining tinyxml2.
I have a kinda brittle workaround for migration in the following PR:
- https://bitbucket.org/ignitionrobotics/ign-common/pull-requests/87/support-new-errorstr-api-in-tinyxml2-600/diff
It only works if I use pkg_check_modules in cmake to find tinyxml2, since that provides the version string. I don't think the cmake config file has version numbers defined.
It was a pretty sweeping change (although long overdue.) I don't generally do the deprecation tick-tock, but...possibly if there is another API consolidation like that.
Macro's would be easy to add. The only trick is modifying the python script. Marking as feature enhancement to remind me.
Thanks. You can rename the issue to focus on adding version number macros if you want.
The define macros are in master; leaving this issue open for a bit. It's good discussion. I'll close if there aren't any additional comments.