Hi Jan.
Thanks for making those changes.
One other nit was introduced this iteration though...
My understanding is that it is bad practice to put bug IDs into code
comments. This was done on line 885 of ict.py regarding the dumpadm
-r. Please remove the reference to 6835106 and file another bug to
document what needs to be fixed after 6835106 is fixed.
No need to send me another review for this change.
Thanks,
Jack
Jan Damborsky wrote:
> Jack Schwartz wrote:
>> Hi Jan.
>>
>> I reviewed your code changes and they look reasonable to me.
>>
>> - I looked at the order that the new entries for
>> update_dumpadm_nodename() are called in install_finish, which seems
>> to be correct when I look at the existing call there for the GUI
>> install.
>>
>> - I'm OK with just pulling out the extra step of a temporary file
>> used to create dumpadm.conf (since this is a new system anyway and
>> dumps anyway would be critical to be able to use the system).
>>
>> My only nit is ict.py / lines 889- 890. Since you're copying
>> dumpadm.conf from a running system to another BASEDIR, I would
>> suggest something like this:
>>
>> dumpadmfile = '/etc/dumpadm.conf'
>> dumpadmfile_dest = self.BASEDIR + dumpadmfile
>>
>> ... but this is only a suggestion.
>
> Hi Jack,
>
> This is a good point. I have changed this according
> to your suggestion and updated the webrev.
>
> Thank you very much for reviewing those changes,
> Jan
>
