angles/2009-02-19 Code Review

Package Developer: Sachin

Examiner: Tully

Present at code review: John

Notes from examiner

Examiner: Enter your notes on each section below. See ExaminerChecklist for more detailed instructions.

  • Divide your concerns up into bullet lists that can be run through in order in the meeting, and concretely decided about. Include enough details / relevant information for people at meeting to quickly decide on the issue (e.g. file and line number, description of race condition, or list unclear names)
  • {X} Use error symbol to call out things that are clear errors

  • /!\ Use warning symbol to call out things that you're worried about

  • (./) Use check mark to mark areas that look good.

  • Use the below bullets as a basic guide, but feel free to add new issues or remove issues that are not relevant (e.g. command-line options for a library)
  • Feel free to delete this instruction section when you're done

Documentation

  • API (./)

  • ROS API (na)
  • command-line (na)
  • tutorials / examples (na)

Naming

  • headers (./)

  • libraries (na)
  • exported symbols (na)
  • ROS API (na)

Source Code

  • header files (./)

    • (./) what happens when input variable to shortest_angular_distance_complement is outside of the specified range [-2*M_PI,2*M_PI]?

      • I added conditioning on input (also renamed to two_pi_complement) --Tully
  • source files (na)
  • data / configuration / xml files (na)

Manifest / Build System

  • build (./)

  • dependencies (./)

  • manifest data (./)

Testing

  • all tests pass (./)

  • unit tests (./)

  • rostests (na)

Conclusions

This section will be filled out during the group code review meeting, and will include

  • /!\ Any action items that need to be taken before clearing

  • Change in official package status
  • There was a bit of a difference between documentation and implementation which was cleaned up yesterday. And unit tests added exposed the problems. Now that that's cleaned up I'm ready to pass it. --Tully
  • It's been two days and we've resolved the few comments. Cleared!!


Wiki: angles/Reviews/2009-02-19_Code_Review (last edited 2009-08-14 20:51:26 by localhost)