On Thu, Sep 13, 2012 at 6:41 AM, tkruse
<tibokruse@googlemail.com> wrote:
Hi,
thanks to Dirk for creating the REP.
My review comments (sorry for providing that many):
* packages could have a namespace (stack), or "group", or similar to give more structure. E.g.: package: amcl, group: navigation
* Attributes "brief" and "email" could be tags as well, based on the rationale given in the REP
I don't think making emails into tags makes much sense, because then there is no relationship between specific author/email pairs, but brief could be a separate tag, but being an attribute might encourage people to make them briefer.
* URL types could be more, and the semantics should be stated when no type is given.
* bugtracker url could have more information, to help later creation of cli tool that creates tickets
* The REP could mention YAML and JSON, and why they were not chosen as a replacement
* The REP could mention which ROS package / library will provide the validation / parsing, to prevent many packages from writing their own parser.
* It might be good to think ahead, that the syntax will change, and declare what version of the package.xml syntax a given package.xml adheres to.
A version attribute for <package> makes sense to me.
* The REP should provide the XML schema for review, not just announce it to be available
* maintainer could be an attribute of author.
Maintainer might not be an author.
* http://ros.org/wiki/Manifest also mentions logo tag (was that ever used?)
* The structure depends, run_depends, build_depends is not future proof, more scopes might become relevant
I like Damon's suggestion of having just <depends> but having an optional scope attribute.
* Also the depends semantics should be clarified, what effect will it have if I put <depends>xyz</depends> in the package.xml?
* The document structure is weird, an additional header after the example might be useful
* build_type should be put in the example
* for the export tag, the valid subtags should be listed
* The naming convention for the name tag differ from: http://www.ros.org/wiki/Naming (lowercase, dashes?, must start with letter)
* dashes in the name???
I agree, when you consider the recommendation to make the containing folder match the name, dashes don't make sense.
* message_generator tag description could be more verbose, also what is the API / contract for such packages?
* REP should mention which tools are known to rely on stack.xml / package.xml (possibly also in what way they will be changed)
* "For catkin packages these files will be
auto generated." sounds weird/redundant, as package.xml files are only supposed to work in catkin packages.
I think he means the manifest.xml files (for automatic backwards compatibility).
* why is the run_depend information used to determine build order? (Copy & paste bug) -> explain what the run_depend tag is really used for
To view this discussion on the web visit https://groups.google.com/d/msg/ros-sig-buildsystem/-/QxPm3ouS5UQJ.