Hi Ethan,
Thanks for the review...
On 18/08/2011 10:17, Ethan Quach wrote:
> Darren,
>
> data_files.py
> ----------------
> 48 - doesn't look like this is used anymore in this file, so it can be
> removed.
>
Removed.
> 866 - place holder for a comment?
Removed.
>
> 890 - Should this 'if' be paired with an else in case system_url is
> None? (The else should perhaps just raise some exception that can be
> caught at 892)
Makes sense, I've added code to raise an exception if the system_url is not
found in the manifest.
>
> 892 - We should probably catch explicit parsing exceptions and output
> the error message(s) accordingly, in case the manifest simply didn't parse.
I added a new message for this.
>
> 916 - This final else clause now catches the exact case where the
> manifest instance the user is trying to use specifies an ai.dtd.X that
> is not equal to the the ai.dtd.Y that is in the imagepath of the install
> service, so let's enhance this error message to say precisely that.
Done.
>
>
> aimanifest.py
> -----------------
> 42 - Doesn't look like this variable definition is used anywhere in this
> file.
Yep, been removed.
I've placed an updated webrev at:
https://cr.opensolaris.org/action/browse/caiman/dkenny/7037014-2/webrev/
Let me know if this is ok,
Thanks,
Darren.
>
>
> -ethan
>
>
> On 08/12/11 10:09, Darren Kenny wrote:
>> Hi,
>>
>> I'd like to request a code review for the bug:
>>
>> 7037014 Install DTDs should be versioned
>>
>> The webrev is at:
>>
>> https://cr.opensolaris.org/action/browse/caiman/dkenny/7037014/webrev/
>>
>> Summary:
>>
>> The basic changes that have been made are as follows:
>>
>> - Rename all .dtd files to .dtd.1
>>
>> - Add references in install_common to get to these file using
>> constants.
>>
>> - Add a method to check if a give string looks like a .dtd or .dtd.1
>> file.
>>
>> - Add constants to Makefile.master for the DTD versions, one for each
>> file.
>>
>> - Generate the .dtd.<Version> file at build time, and similarly, any
>> file that needs a reference to this versioned name.
>>
>> For this to work I've added an entry in Makefile.master to do a sed
>> on any file with a .src suffix.
>>
>> The sed replaces any @DTD_VERSION_XX@ type values in the text.
>>
>> All .xml files are now named .xml.src.
>>
>> - The packaging refers to variables $(DTD_VERSION_XX), which are
>> replaced as as part of the pkgmogrify.
>>
>> - Symlinks exist now for ai.dtd and dc.dtd to their versioned
>> counterpart. This is to aid migration at this last stage.
>>
>> - Attempting to add an XML manifest with a reference to ai.dtd will
>> generate an error if there is an ai.dtd.1 file also in the install
>> image.
>>
>> Testing done:
>>
>> In all my generation of ISOs I've been using the DC in the build I'm
>> doing, so DC has been well tested.
>>
>> Jack did some Derived Manifest testing (Thanks)
>>
>> AI installs have been done using various AI servers:
>>
>> - snv_151a
>> - snv_171
>> - snv_171 + dtd_versioned bits
>>
>> Both SPARC and x86 clients were tested - with the AI clients installing
>> the ISOs generated from the dtd_versioning workspace and snv_171 ISOs.
>>
>> I also verified that an zones install worked too.
>>
>> Thanks,
>>
>> Darren.
>> _______________________________________________
>> 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
