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
>
> --
> You received this message because you are subscribed to the Google Groups
> "ROS Buildsystem Special Interest Group" group.
> To post to this group, send email to ros-sig-buildsystem@googlegroups.com.
> To unsubscribe from this group, send email to
> ros-sig-buildsystem+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msg/ros-sig-buildsystem/-/QxPm3ouS5UQJ.
>
> For more options, visit https://groups.google.com/groups/opt_out.
>
>
>
--
William Woodall
Willow Garage - Software Engineer
wwoodall@willowgarage.com
(text/plain)