Libguestfs - Aug 2016 - [PATCH] v2v: Use unitless methods for methods which don't change the internal state.

Methods in OCaml which don't take any parameters don't require the
dummy unit arg, ie writing:

  method foo = ...

is fine.  The reason you might need the unit arg is if you need to
create a closure from the method without calling it, for example if
you need to use the method in a callback.

In lablgtk2 the convention is to use unitless methods if either: the
method shouldn't be used as a callback; or: (conceptually) the method
doesn't change the object's internal state.  Let's do that here.
---
 v2v/convert_linux.ml      |  2 +-
 v2v/linux_bootloaders.ml  | 14 ++++++--------
 v2v/linux_bootloaders.mli |  2 +-
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
index 7829612..e1a769b 100644
--- a/v2v/convert_linux.ml
+++ b/v2v/convert_linux.ml
@@ -240,7 +240,7 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect
source rcaps     * list is the default booting kernel.
    *)
   let grub_kernels : kernel_info list -    let vmlinuzes =
bootloader#list_kernels () in
+    let vmlinuzes = bootloader#list_kernels in
 
     (* Map these to installed kernels. *)
     filter_map (
diff --git a/v2v/linux_bootloaders.ml b/v2v/linux_bootloaders.ml
index d12edf8..f41947e 100644
--- a/v2v/linux_bootloaders.ml
+++ b/v2v/linux_bootloaders.ml
@@ -29,7 +29,7 @@ module G = Guestfs
 class virtual bootloader = object
   method virtual name : string
   method virtual augeas_device_patterns : string list
-  method virtual list_kernels : unit -> string list
+  method virtual list_kernels : string list
   method virtual set_default_kernel : string -> unit
   method set_augeas_configuration () = false
   method virtual configure_console : unit -> unit
@@ -69,7 +69,7 @@ object
     "/files/etc/sysconfig/grub/boot";
   ]
 
-  method list_kernels () +  method list_kernels      let paths        let expr
= sprintf "/files%s/title/kernel" grub_config in
       let paths = g#aug_match expr in
@@ -189,7 +189,7 @@ class bootloader_grub2 (g : G.guestfs) grub_config  object
(self)
   inherit bootloader
 
-  method private grub2_update_console ~remove +  method private
grub2_update_console ~remove ()      let rex = Str.regexp
"\\(.*\\)\\bconsole=[xh]vc0\\b\\(.*\\)" in
 
     let paths = [
@@ -235,7 +235,7 @@ object (self)
     "/files/boot/grub2/device.map/*[label() !=
\"#comment\"]";
   ]
 
-  method list_kernels () +  method list_kernels      let get_default_image ()  
let cmd          if g#exists "/sbin/grubby" then
@@ -285,11 +285,9 @@ object (self)
           " vmlinuz |] in
     ignore (g#command cmd)
 
-  method configure_console () -    self#grub2_update_console ~remove:false
+  method configure_console = self#grub2_update_console ~remove:false
 
-  method remove_console () -    self#grub2_update_console ~remove:true
+  method remove_console = self#grub2_update_console ~remove:true
 
   method update ()      ignore (g#command [| "grub2-mkconfig";
"-o"; grub_config |])
diff --git a/v2v/linux_bootloaders.mli b/v2v/linux_bootloaders.mli
index cf115e3..3500415 100644
--- a/v2v/linux_bootloaders.mli
+++ b/v2v/linux_bootloaders.mli
@@ -21,7 +21,7 @@ class virtual bootloader : object
   (** The name of the bootloader. *)
   method virtual augeas_device_patterns : string list
   (** A list of Augeas patterns to search for device names. *)
-  method virtual list_kernels : unit -> string list
+  method virtual list_kernels : string list
   (** Lists all the kernels configured in the bootloader. *)
   method virtual set_default_kernel : string -> unit
   (** Sets the specified vmlinuz path as default bootloader entry. *)
-- 
2.7.4

On Fri, Aug 26, 2016 at 02:08:37PM +0100, Richard W.M. Jones
wrote:
> In lablgtk2 the convention is to use unitless methods if either: the
> method shouldn't be used as a callback; or: (conceptually) the method
> doesn't change the object's internal state.  Let's do that
here.
I finally found the reference for that:

http://lablgtk.forge.ocamlcore.org/README.html

"Note about unit as method argument"

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

On Friday, 26 August 2016 14:08:37 CEST Richard W.M. Jones
wrote:
> Methods in OCaml which don't take any parameters don't require the
> dummy unit arg, ie writing:
> 
>   method foo = ...
> 
> is fine.  The reason you might need the unit arg is if you need to
> create a closure from the method without calling it, for example if
> you need to use the method in a callback.
> 
> In lablgtk2 the convention is to use unitless methods if either: the
> method shouldn't be used as a callback; or: (conceptually) the method
> doesn't change the object's internal state.  Let's do that
here.
> ---
Ah OK, it makes sense.  I chose the safe way and added the unit arg
to avoid issues with closures, although I'm fine with the change given
that it follows an established convention.

Thanks,
-- 
Pino Toscano