Reviewer: Wim Meeussen
Instructions for doing a doc review
See DocReviewProcess for more instructions
- Does the documentation define the Users of your Package, i.e. for the expected usages of your Stack, which APIs will users engage with?
- Are all of these APIs documented?
- Do relevant usages have associated tutorials? (you can ignore this if a Stack-level tutorial covers the relevant usage), and are the indexed in the right places?
- If there are hardware dependencies of the Package, are these documented?
- Is it clear to an outside user what the roadmap is for the Package?
- Is it clear to an outside user what the stability is for the Package?
- Are concepts introduced by the Package well illustrated?
- Is the research related to the Package referenced properly? i.e. can users easily get to relevant papers?
- Are any mathematical formulas in the Package not covered by papers properly documented?
For each launch file in a Package
- Is it clear how to run that launch file?
- Does the launch file start up with no errors when run correctly?
- Do the Nodes in that launch file correctly use ROS_ERROR/ROS_WARN/ROS_INFO logging levels?
Concerns / issues
- On the main wiki page, class names do not start with a capital:
PLUGINLIB_REGISTER_CLASS(rectangle, rectangle_namespace::rectangle, polygon_namespace::polygon)
while I would expect:
PLUGINLIB_REGISTER_CLASS(Rectangle, rectangle_namespace::Rectangle, polygon_namespace::Polygon)
Tully: Fixed capatalization of classes.
- It is not always 100% clear if a name is a package/namespace/class. Why not make the names explicit:
pluginlib::ClassLoader<polygon> poly_loader("polygon_interface", "polygon_namespace::polygon");
pluginlib::ClassLoader<PolygonClass> poly_loader("polygon_package", "polygon_namespace::PolygonClass");
E.g. a few lines lower, there is the line:
poly = poly_loader.createClassInstance("rectangle");
It is not clear if "rectangle" is the package name, the class name or even something else?
Tully: replaced polygon_interface with polygon_interface_package for clarity, and the caps on the class name for clarity.
- The tutorials page only contains a number of dead links, no actual tutorials.
Tully: broken links removed
- Roadmap and stability are not mentioned. Maybe just add a few lines in the manifest package description
Tully: yes, fixed
Are multiple <export> elements allowed in the manifest? If not, it would be good to mention this in the "Exporting a Plugin" section on the main wiki page.
Tully: Noted in both the main page and the exports subpage.