[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [virt-tools-list] [PATCH] Add desktop profile for Windows



On Fri, Aug 17, 2012 at 12:30 PM, Zeeshan Ali (Khattak)
<zeeshanak gnome org> wrote:
> On Fri, Aug 17, 2012 at 5:40 AM, Fabiano FidĂȘncio <fabiano fidencio org> wrote:
>> To create an user and set their avatar, we need to create 2 new files,
>> that will be used in older Windows.
>> (http://bugzilla-attachments.gnome.org/attachment.cgi?id=214681)
>>
>> Was discussed in the ML about don't create this files and put all
>> information in only one script, separating each file in one template, as
>> suggested in:
>> https://www.redhat.com/archives/virt-tools-list/2012-June/msg00078.html
>
> It starts with "I wonder" and it doesn't seem to be addressed in the
> thread later. I think most of this conversation (unfortunately)
> happened on IRC and IIRC we either agreed to have multiple script
> files or decided to change the _install_script_generate*() to return
> multiple files.
>
> Daniel, do you recall?

In any case, the patch itself looks good to me so ACK from my side
except for 1 minor change, I'd suggest:

+        if (osinfo_install_script_generate_output(script,
+                                                  os,
+                                                  config,
+                                                  dir,
+                                                  NULL,
+                                                  &error) == NULL) {

If we are going to only use the return value to check for errors, I
think it would be slightly more cleaner to use
osinfo_install_script_generate() instead and instead of checking for
its return value, we check for 'error != NULL' after the call.

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]