rospack/2008-09-04 Code Review

Package Developer: Multiple. Brian pushing through review

Reviewers:

• Brian
• Ken
• Morgan
• Rob

Code Status

1. Code is accompanied by thorough tests

   1. Unit tests for code
      • Backquote evaluation test

        • Ken requests more complex test (done)

      • deps - check dependencies (including checking ordering and formatting everywhere)
      • deps_higher - check to make sure single-depth deps work *deps_invalid - make sure rospack fails on invalid dependency
      • depends_on - test case
      • lflags_backquote - ensure basic well-formed single lib works (redone with escaped backquoting)
      • lflags_deps - ensure flags from dependencies are properly ordered
      • lflags-deps-only - ensure that we get back ordered flags without the package's flags itself
      • sysdeps_invalid - ensure that bogus xml causes failure. Also tests that invalid xml in the system doesn't crash other tests
      • other commands on invalid xml

      • no tests for invalid manifests that are valid manifests

      • vcs0_deps - make sure empty version control is returned OK
      • vcs_deps - make sure we get version control tags from dependencies properly

      • no sysdeps0

      • sysdeps_base - make sure sysdeps works properly

      • Meaner tests requested

      • ros package path

        • multiple paths (done)

        • invalid locations (done)

      • circular dependencies (done)

      • autogenerated depth 100 list (done)

      • depends-on1 (done)

      • find (done)

      • list (done)

      • langs

      • cflags tests

      • export

      • repeated fields (name, license, etc.)

      • not setting ros-root (done)

      • invalid ros-root (done)

   2. Integration tests for ROS nodes
      • N/A
   3. Code coverage
      • Not measured. Not complete.

2. API is stable and matches naming conventions

   • N/A

3. All code is documented.

   • Undocumented options (predeps)

   • Generally documented
4. Implementation review

   • Command-line parsing is spread out, hand coded, and not unified

   • Ordering of flags is currently required

   • Syntax of export would need to change to clean up parsing

Minutes

Should nag on invalid manifests need to remove rospack make from code and help

Command-line syntax and unification

• --option vs command

• remove "--" before foo-only-bar commands

• export syntax is different from other commands

Possible command-line solution

• Command, --options, and package name

Conclusion

• Needs more robust testing
• Needs removal of old options and change of syntax
• Needs update of command string
• Command-line testing passes tests, but is not extensible.