Hi Drew,
        Thank you for reviewing this! Some comments in-line, anything you 
sent not mentioned below, I'll be fixing.
                                        Thank you,
                                        Clay

On Tue, 8 Sep 2009, Drew Fisher wrote:

> Code review is attached.
>
> Please let me know if you have any questions or need more information.
>
> -Drew
>

>Consider replacing all of the grubMenu class with calls to bootadm.
>There's a metric ton of undocumented features in bootadm (ask Bill K.)
>that can probably do what you need without having to wrap the whole
>thing in a class.

I've sent Bill a ping but I'm still not fond of the idea of using bootadm 
undocumented features when the Grub file format is a published spec which 
needs to be adhered to.

>The entire remove_files () function just smacks of poor architecture.
>You're reverse engineering the entire system?  Why?  Can't you simply
>query the SMF service to find all the information you need regarding
>these files and if they're unique or being used elsewhere?  If not, I
>would recommend pushing up higher and get the SMF service augmented to
>support this kind of information.  It will take a 400 line function
>and reduce it to something around 10 lines.

This is an unfortunate affect of installadm's current design we are 
working through. I'm happy to see others see the current architecture as 
broken not storing such necessary configuration details of an AI service.

Reply via email to