Skip to content
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

msa: cleanup yaml parsing #795

Merged
merged 3 commits into from
Mar 26, 2018
Merged

Conversation

rhaschke
Copy link
Contributor

While tracking down a bug in yaml parsing, which finally turned out to be a bug in yamp-cpp itself (jbeder/yaml-cpp#568), I cleaned the code a little bit.

@@ -129,8 +127,7 @@ void operator>>(const YAML::Node& node, T& i)
typedef const YAML::Node* yaml_node_t;

// Helper function to find a value (yaml-cpp 0.3)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we just remove support for yaml-cpp 0.3 at this point?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yaml-cpp 0.3 is default for Ubuntu 14.04 / ROS Indigo. In the Kinetic branch, indeed we could get rid of 0.3 support.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@rhaschke
Copy link
Contributor Author

@davetcoleman I removed yaml 0.3 support and further simplified the yaml parsing code.

- limit findValue to std::string key
  using templates for char[N] is ineffient, as every literal of new
  length N requires a new template instance
- use boost::optional constructor to return value
Copy link
Member

@davetcoleman davetcoleman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm except clang-format is now failing

@@ -129,8 +127,7 @@ void operator>>(const YAML::Node& node, T& i)
typedef const YAML::Node* yaml_node_t;

// Helper function to find a value (yaml-cpp 0.3)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

- using utility function parse()
- removing yaml-cpp 0.3 support
@rhaschke rhaschke merged commit 0ba9bd3 into moveit:kinetic-devel Mar 26, 2018
@rhaschke rhaschke deleted the msa-yaml-parsing branch March 26, 2018 07:31
sjahr pushed a commit to sjahr/moveit that referenced this pull request Jun 21, 2024
Signed-off-by: Gaël Écorchard <gael.ecorchard@cvut.cz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants