[Libguestfs] [PATCH v2] v2v: factor out bootloader handling

Pino Toscano ptoscano at redhat.com
Fri Aug 26 08:21:29 UTC 2016


On Thursday, 25 August 2016 17:36:14 CEST Richard W.M. Jones wrote:
> On Thu, Aug 25, 2016 at 06:05:16PM +0200, Pino Toscano wrote:
> > +(* Grub2 representation. *)
> > +class bootloader_grub2 (g : G.guestfs) grub_config =
> > +  let grub2_update_console ~remove =
> 
> I checked the before and after code and I don't think anything has
> been missed out.
> 
> My only comment is it seems a bit awkward stuffing the
> grub2_update_console function definition into what is effectively the
> class constructor.  Maybe it should be hoisted to the top level of the
> Linux_bootloaders module (not exported, of course), or turned into a
> private method of the class?

Indeed -- I turned that into a proper private class method, so it is
not available outside the bootloader_grub2 class.

> Anyway as this is just a quibble, ACK.

Quibbling is fine :)

Pushed with the aforementioned change.

Thanks for the thorough review,
-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20160826/50ffca8f/attachment.sig>


More information about the Libguestfs mailing list