search

ros / urdfdom

URDF parser
http://ros.org/wiki/urdf
Other
97 stars 131 forks source link

Remove tinyxml2 from public API #189

Closed scpeters closed 9 months ago

scpeters commented 9 months ago

We are upgrading from tinyxml to tinyxml2 in https://github.com/ros/urdfdom/pull/186 (resolving https://github.com/ros/urdfdom/issues/185), which will be included in the next major release. An alternate approach to replacing tinyxml2 in https://github.com/ros/urdfdom/pull/178 also made a change to redact tinyxml2 from the public API, so we should apply that change as well after #186 is merged.

moooeeeep commented 9 months ago

Did you consider to try to hide the tinyxml2 types with a typedef? Maybe this would help client code with the next upgrade, as they would just continue to use urdf::XMLDocument.

(untested) Example:


namespace urdf{
  using XMLDocument = tinyxml2::XMLDocument;
  ...
  URDFDOM_DLLAPI XMLDocument*  exportURDF(ModelInterfaceSharedPtr &model);
  ...
}
clalancette commented 9 months ago

Did you consider to try to hide the tinyxml2 types with a typedef? Maybe this would help client code with the next upgrade, as they would just continue to use urdf::XMLDocument.

Yeah, we could do that. Honestly, I suspect that all of the APIs that take tinyxml2 APIs are unused, so I'm considering marking them deprecated and eventually removing them.