Code reviewer checklist

To prepare for a design review meeting:

• Go to the official wiki page for the package (pr.willowgarage.com/wiki/package_name). If this page doesn't exist or has a different name, that is a bug and should be fixed. This link is used for autogenerated links from rosdoc, so must be correct.
• Click the "Code Review" date and type the date (yyyy-mm-dd) and your name to create a page to take notes on this process. This page will be used as the basis for a code review meeting, so your comments should have enough detail that you will feel comfortable presenting them to 3-6 other developers.
• Run through the steps below, documenting your comments and thoughts for the code review.

Expectations

• This process will take significant time. Depending on the size of the package, this process could take from several hours to several days.
• Raise concerns. You are reviewing the code, not the coder, so don't "let it slide" if there's something you're worried about. Those decisions can be made as a group at the code review. Your job is to raise issues so that the full group doesn't have to run through this process.
• Short-circuit if something is significantly wrong. If you find significant problems, feel free to close the loop with the developer, or recommend that the review be postponed. Since you're putting in a lot of time on this, we want to make sure it's as efficient as possible

Documentation

Read through rosdoc documentation on package

• Are all APIs well-documented?
• Are all command-line arguments well-documented?
• Are all ROS topics, services, and parameters well-documented?

Check wiki for tutorials and examples linked from package page

• What tutorial and examples are linked from the page?
• Do they cover the right things?
• Are there any that seem to be missing?

Naming

Check that externally visible names match naming conventions

• Are header files well named?
• Are libraries well named?
• Are externally visible classes, functions, methods, namespaces, and variables well named?
• Are all ROS topics, services, and parameters well named?

Source code

Read all source files, looking for and noting down problem areas (where applicable, mark down file and line number for easy review in meeting)

• are there coding or design issues that are worrisome to you
• is ros::Time used (no gettimeofday or nanosleep)?
• is rosout used for error/warning messages? (no printf)
• are transforms being done with tf?
• are major assumptions and magic thresholds documented (and necessary)?
• is variable data derived from the correct source? (e.g., robot kinematics come from pr2.xml)
• is code unnecessarily PR2-specific or ROS-specific when it should be more general?
• is there platform dependent code?
• are there possible race conditions?

Manifest / build system

Build the package and read through the build output

• did it build?
• are there incorrect, unnecessary, or missing dependencies?
• are there compiler warnings that originate in this package's code? (not from #included code)

Testing

Read unit test code and rostest .xml files. Type make test to run tests.

• do all defined tests actually run?
• do all the tests pass?
• do the unit tests sufficiently test the code at a library level?
• do the rostests sufficiently test the code at a ROS level?