Re: [Libguestfs] [PATCH v2v 1/3] qemu-nbd: Implement output compression for qcow2 files

On Tue, Jul 05, 2022 at 08:51:49AM +0200, Laszlo Ersek wrote: > On 07/01/22 13:01, Richard W.M. Jones wrote: >> --- >> lib/qemuNBD.ml | 11 +++++++++-- >> lib/qemuNBD.mli | 5 +++++ >> output/output.ml | 38 +++++++++++++++++++++++++++++++++++--- >> output/output.mli | 1 + >> 4 files changed, 50 insertions(+), 5 deletions(-) > > (Can you update your git diff order so that *.mli be put before *.ml?) In nbdkit we have scripts/git.orderfile: https://gitlab.com/nbdkit/nbdkit/-/blob/master/scripts/git.orderfile I guess we should copy this into other projects. ... >> + let () = > > What is the purpose of "let ()" here specifically? > > (I know what it's good for in general, from your earlier reference > <https://baturin.org/docs/ocaml-faq/>;, but I don't remember seeing it > much outside of the outermost scope.) > >> + let version = NBD.create () |> NBD.get_version in >> + let version = String.nsplit "." version in >> + let version = List.map int_of_string version in >> + if version < [1; 13; 5] then It hides the binding of "version", so it doesn't escape from this scope. It's not necessary, but avoids possible confusion if we used an unrelated "version" symbol somewhere later in the code.

> (Huh, didn't know such comparison existed, great!) Sure, you can compare anything except recursive structures, functions (because of the Halting Problem IIRC) and certain peculiar internal types. It's nothing clever, it just recursively compares the two structures until it finds something unequal: https://github.com/ocaml/ocaml/blob/d9afa408c612e74a266b95f0fa25bb1efde72...

> Reviewed-by: Laszlo Ersek <lersek(a)redhat.com&gt; Thanks - I'll address the other comments you made in the updated patch. Rich.