Hi everyone.

Here are my round-2 comments:

auto-install/ai_manifest.xml:
-----------------------------

Nit: 36-37: get rid of "in the AI schema." It is too much info and is not needed.

192: ... full path is in the source/publisher/origin name.
Move 195 to just above 203
205: The source/publisher/origin name corresponds to the directory...
206: software/software_data/name refers to the package name subdirectory...
Move 211-212 to just above 219
222: The source/publisher/origin name refers to the path...
Move 227 to just above 233

default.xml:
------------

The <add_drivers> section is missing...

It can go between lines 49-50:

<!--
            Uncomment before using
<add_drivers>
<search_all/>
</add_drivers>
        -->

software.dtd
------------

Nit: 79: A facet is an option...

auto_ddu_lib.c: OK!  (Thanks, Dermot, for all of your work on this!)
---------------

auto_parse.c:
-------------

770: Nit: Say "191" to represent a Solaris partition.

install_utils/ManifestServ.py:
------------------------------

393, 459: Update comments that ManifestServError can be raised.

The rest looks OK to me.

    Thanks,
    Jack




On 08/16/10 12:55, Sue Sohn wrote:
Thanks to everyone for your comments. Below are pointers to updated and incremental webrevs which address those comments. We'd appreciate a quick turnaround on this one so that we can finish the review by COB Wednesday, 8/18.

Webrev:
-------
 Full webrev:  http://cr.opensolaris.org/~sohn/webrev.ai-schema.v2/
Incr. webrev:  http://cr.opensolaris.org/~sohn/webrev.ai-schema.v2.incr/

Note that the incremental webrev did not pick up the diffs for the renamed publish_manifest.py file and instead shows all changes from this project.

Thanks,
Sue


On 08/05/10 15:07, Ethan Quach wrote:
Hi all,

The following is the code review for the AI manifest schema changes,
and the installadm criteria changes.  It is a rather large review, so
partial/piecewise review would be also be fine, just let us know what
you're reviewing.  We've pre-requested reviews from some of you
already, but all comments welcomed by Aug 16th.


Webrev:
-------------
http://cr.opensolaris.org/~equach/webrev.ai-schema/

Bugs:
--------
16423 <http://defect.opensolaris.org/bz/show_bug.cgi?id=16423> Updates to AI schema should be made 15449 <http://defect.opensolaris.org/bz/show_bug.cgi?id=15449> installadm add validates combined manifest against image-specific schema as well as schema in /usr/share/auto_install/ 6975043 <http://bugs.opensolaris.org/view_bug.do?bug_id=6975043> separate criteria and ai manifest



thanks,
-ethan


------------------------------------------------------------------------

_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss


_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

Reply via email to