Hi Jean,
Thanks for the review! My responses are in-line.
* jeanm (Jean.McCormack at Sun.COM) wrote:
> DC-manifest.rng:
> Lines 59-68: Does this mean that we now could have neither
> nm_img_params or nm_vming_params? It just
> looks to me like you modified the requirements for a live image
> build. Can you explain?
Somehow I missed this when I was reviewing the webrev prior to publish.
There actually aren't any vmimg_params for VMC. Everything is set as
arguments to the VMC finalizer scripts. So the block 52,73 should look
like:
<attribute name="name"/>
<!-- General distro-constructor parameters -->
<ref name="nm_distro_constr_params"/>
! <optional>
! <!-- Parameters for building live
image -->
<ref name="nm_img_params"/>
+ </optional>
The effect of which makes nm_img_params optional for all manifests since
it's not needed for VMC manifests (and there was no good way I could
come up with to make nm_img_params mandatory for the AI/SLIM manifests
but not for VMC manifests).
> lines 65-68: My assumption is that you uncomment this if you want
> it? Correct? I believe the following comment does with this one.
> lines 294-307: Haven't you made the definition of nm_vming_params a
> big comment?
So, along with my prior comment, lines 294-307 are removed as well.
> DC_checkpoint.py:
> Not PEP8 compliant. Feel free to call me since I've spent some time
> making some DC files so.
Yep. We'll clean this up prior to integration.
>
> DC_tm.py -> im_pop.py:
> It looks like you mostly copied DC_tm.py to im_pop.py. If so, it's
> easier for people to see the real
> changes if you do a hg rename DC_tm.py im_pop.py
Good idea. Thanks.
> distro_const.py:
> Not PEP8 compliant.
Again, something we'll clean up.
> im_pop.py:
> Not PEP8 compliant
Really? It verifies fine for me in netbeans (I specifically tried to
make this PEP8 compliant). How is it not?
> vmc shell scripts:
> I'll leave these for others
Fair enough. Thanks Jean!
--
Glenn
