Hey Dave, Thanks again for the review feedback.
The updated webrev can be found here: http://cr.opensolaris.org/~joev/bug15163_r2/ My comments are below. Thank you for the review. Joe On 03/15/10 01:55 PM, Dave Miner wrote: > On 03/12/10 03:52 PM, Joseph J VLcek wrote: >> Please review the fixes for: >> >> Bug 15163 - DC needs to be enhanced to publish the AI image as an IPS >> repository >> >> The webrev is here: >> http://cr.opensolaris.org/~joev/bug15163/ >> > > ai_publish_pkg: > > 232: Can we make the argument list optional? Since defaults all work, > this should be possible. Done. > > 327: Version should not be taken from the execution environment, it > should come from the image being published. > Done > 345: Why wouldn't we default the publisher to opensolaris.org? > I used: ai.opensolaris.org If we used opensolaris.org it could clash with that publisher named already in use. > 357: Is this actually necessary? Won't the checkpointing code have taken > care of it? remove this cleanup > > 370: I was surprised that this isn't encapsulated in an eval in order to > get the transaction id into the environment; perhaps that's not > necessary for file: FMRI's? However, I'd expect it's necessary to > support http: FMRI's, which would be nice to allow, so does it break > anything to have the transaction id? Obviously, if we do support http: > FMRI's, then the comments in the manifests need updating. Reworked this code with input from Shawn Walker.. > > 389-391: Instructional output doesn't seem necessary removed > > Dave Thanks! Joe
