camera_info_manager/Reviews/2011-07-12_Doc_Review
Proposer: Jack O'Quin
Reviewers:
- Ken Conley
- Eric Perko
- Chad Rockney
Documents to be reviewed
camera_info_manager::CameraInfoManager class
The API review for this package in this release was held on 2011-06-06.
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?
Concerns / issues
- kwc:
- Overall, very good (reviewing as a non-user)
- I would consider deleting the 'tutorial' and just having an 'examples' section. I added a proposed examples section to the front page
(Eric) +1 . Can the "tutorial" link in the "Package Links" be changed to point to an examples page?
(kwc) No, but I recently updated the macro so it will only show the Tutorials page if it exists (i.e. no more gray links)
(Jack) I am inclined to keep the Tutorial page, new users are likely to look there (see below).
- I might remove the swissranger example as it's not updated to the non-deprecated API, correct?
(Jack) Good catch. I agree. I considered updating that package (it's only a two-line fix), but then realized that it is no longer the simple example that it had been, anyway. Plus that package is not very actively maintained, so probably not a good example.
- (FIXED): example for camera1394 linked to old camera1394.cpp, updated to driver1394.cpp instead.
in the wiki page, I would consider linking directly to the doxygen instead for the URL documentation in order to avoid consistency issues -- it's also the case that the wiki and doxygen docs add slightly different information, so it would be good to make one authoritative. Sample rewrite on wiki: "The location for getting and saving calibration data is expressed by Uniform Resource Locator (URL). These URLs are commonly used in the APIs of this package and may also contain substitution variables to refer to common locations. Please see the CameraInfoManager API documentation for supported URLs and substitution variables."
(Eric)
- Things look pretty good. It was a bit difficult to pick out the new stuff for Electric though - the yellow highlights weren't doing a good job for me since there were some for Diamondback and some for Electric. Did that versioning macro (that switched the entire page) go away? Or could we remove the "new in Diamondback" marks?
(Jack) I prefer to delete the Diamondback marks; they're not all that new anymore. The macro to switch the whole page requires maintaining multiple versions. This package is not complicated enough to need that.
- Things look pretty good. It was a bit difficult to pick out the new stuff for Electric though - the yellow highlights weren't doing a good job for me since there were some for Diamondback and some for Electric. Did that versioning macro (that switched the entire page) go away? Or could we remove the "new in Diamondback" marks?
(Chad) I used this for the bare minimum of functionality last week. The links to example code in the tutorials section were the useful. The other most useful doc was the example URLs on the main page.
(Jack) Useful feedback. I still believe camera_info_manager needs a Tutorial, but can't promise to write one any time soon. Keeping an (up-to-date) stub version with examples seems worthwhile, because that is probably the first place new users will look.
Conclusion
Make doxygen comments the authoritative specification of the current C++ interface. Refer to that specification in wiki page.
Remove Diamondback version marks.
Remove swissranger example.
Keep the Tutorial stub, using the camera1394 driver as an example.
Write a simple example specifically for the Tutorial.