Re: [Libguestfs] [PATCH v2] v2v: factor out bootloader handling
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
Attachments:
- signature.asc (application/pgp-signature — 819 bytes)