xacro/Reviews

Package proposal

Fill this section in with your proposal for the package, and send the link to this page out for discussion.

Keep this section up to date over the course of the email conversation until the proposal is accepted.

When appropriate, add a parallel "API proposal" section

Comments

kwc

Overall, pretty good. It answers my original complaints with the pr2.xml syntax with respect to templates not really being templates as they did not have variables. This is very similar to the Apache ant spec, at least with respect to defining and instantiating variables. The main things I would consider:

  • Removing the 'def' prefix. Everything is a definition -- the macros and the const aren't special in this regard.
  • Consider putting the Xacro tags in a xacro namespace, if we expect to be able to use Xacro as a pre-processor for other ROS/PR2-related XML files.
  • Possibly rename 'const' to 'property' so that it will, in fact, be identical to the Apache ant syntax. Regardless, it should only be called 'const' if it is truly const -- i.e. if it can have a default value that can be overridden, then it should not be a const.
    • One argument against calling it 'property' is that the Apache 'property' tag also allows 'location', 'url', 'file', and 'resource' attributes to initialize instead.

Package review meeting notes

  1. xacro/Reviews/2009-08-06_API_Review
  2. xacro/Reviews/2009-09-23 API Review
  3. xacro/Reviews/2009-10-06_Doc_Review

Create new package review

Enter the date in the appropriate box to generate a template for an API or code review meeting. Check the QAProcess page for more information.

Wiki: xacro/Reviews (last edited 2009-10-06 20:34:57 by TullyFoote)

Except where otherwise noted, the ROS wiki is licensed under the
Creative Commons Attribution 3.0