-
Notifications
You must be signed in to change notification settings - Fork 79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch from TinyXML to TinyXML2 #35
Conversation
The library TinyXML is considered to be unmaintained and since all future development is focused on TinyXML2 this patch updates urdfdom_headers to use TinyXML2. Signed-off-by: Dmitry Rozhkov <[email protected]>
TinyXML is used in public functions that are used in Ideally, the existing functions should be deprecated and the new TinyXML2 using functions should be introduced alongside the existing deprecated functions. However given the extend to which TinyXML is used in the public API, and (as far as I know) the fact that there is no runtime compatibility layer between TinyXML and TinyXML2, I don't think this is even possible. A possible approach is to create a new totally incompatible urdfdom (major?) version, but then all the downstream projects will need to migrate from this version of urdfdom to the "TinyXML2" power version. |
since it's maintained better than deprecated TinyXML. The patch set is not going to be merged to the upstream ROS layer because only ROS Indigo release is maintained in the layer currently. In ros/rospack#62 it was suggested that the packages in the ROS Indigo release are not going to switch to TinyXML2, but it may happen in future releases. Also a comment in ros/urdfdom_headers#35 suggests that the switch is precluded by the fact TinyXML types are widely used in urdfdom's public API thus requires fixing all downstream packages which can be difficult if possible at all. Signed-off-by: Dmitry Rozhkov <[email protected]>
If we were going to make a big change like this, I would prefer to hide the tinyxml / tinyxm2 symbols from the public interface entirely. Perhaps pass a string instead of a tinyxml object? |
Does anybody know who exactly uses this API? Googling doesn't reveal any code using urdf::World::initXML(). |
since it's maintained better than deprecated TinyXML. The patch set is not going to be merged to the upstream ROS layer because only ROS Indigo release is maintained in the layer currently. In ros/rospack#62 it was suggested that the packages in the ROS Indigo release are not going to switch to TinyXML2, but it may happen in future releases. Also a comment in ros/urdfdom_headers#35 suggests that the switch is precluded by the fact TinyXML types are widely used in urdfdom's public API thus requires fixing all downstream packages which can be difficult if possible at all. Signed-off-by: Dmitry Rozhkov <[email protected]>
since it's maintained better than deprecated TinyXML. The patch set is not going to be merged to the upstream ROS layer because only ROS Indigo release is maintained in the layer currently. In ros/rospack#62 it was suggested that the packages in the ROS Indigo release are not going to switch to TinyXML2, but it may happen in future releases. Also a comment in ros/urdfdom_headers#35 suggests that the switch is precluded by the fact TinyXML types are widely used in urdfdom's public API thus requires fixing all downstream packages which can be difficult if possible at all. Signed-off-by: Dmitry Rozhkov <[email protected]>
I don't think the |
See #39 where that function is just removed. |
The library TinyXML is considered to be unmaintained and since all future development is focused on TinyXML2 this patch updates urdfdom_headers to use TinyXML2.