[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