Evan,
On 08/17/10 23:53, Evan Layton wrote:
On 8/16/10 1:55 PM, 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/
I concentrated on the code I had looked at before...
nit:
in auto_ddu_lib.c the first time I looked at line 1071 I didn't read
line 1072 with it and got "publisher and origin are both optional, but
both must be specified". Which made me go hmmm... ;) Could we change
this to something more like "publisher and origin are both optional,
but if one is specified the other must also be used". OK go ahead and
laugh at me now ;)
No, that's a reasonable change. I've fixed it.
nit:
Has this been run through cstyle (etc) it appears like there may be a
few lines longer than 80 characters. (see line 97 in auto_parse.c)
Fixed this and all the other cstyle nits from auto_parse.c.
nit:
in auto_parse.c line 1067
* The name (number) and action attributes are mandatory and
slices, so
was this supposed to be "for slices" instead of "and slices"?
Correct. Fixed.
Other than the minor nit I mentioned before with respect to the
pointer initialization it appears that all my previous comments have
been addressed.
Looks good to me. Ship it! ;)
Thanks!
- Dermot
-evan
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
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
