Re: [Libguestfs] [PATCH 0/6] generator: Split up generator/actions.ml
On Tue, Feb 21, 2017 at 02:47:29PM +0100, Pino Toscano wrote:
On Saturday, 18 February 2017 12:54:35 CET Richard W.M. Jones wrote:
> Split up the huge generator/actions.ml into several smaller files.
Few notes:
- regarding patch #2: wasn't the order stable before? Except from the
explicitly seen order change in tests_c_api.ml, what made the actions
order non-deterministic?
The tests were previously ordered by something like
non_daemon_functions @ daemon_functions with no sorting at all. Which
was a stable order, however it's difficult to replicate that exactly
in the new code because previously interleaved actions are now moved
into various submodules and the submodules are concatenated.
What I was actually trying to achieve with this patch was to make sure
that I could compare the output of the generator before and after the
entire sequence of commits (from patch #3) was applied, so I could
check that it didn't change (except for stable_uuid which changed
because the contents of the actions_*.ml files changed). Indeed there
weren't any changes, which was how I verified the patch set. But for
that to work I had to have a stable ordering that I could reproduce in
the new code.
- regarding patch #5: download_inode should be in actions_tsk.ml
OK, will fix.
- regarding patch #5: I'm personally not fond of the
_deprecated.ml
versions: that means in case an API is deprecated, then it is moved
from one place to another, IMHO for no reason.
+1 for the split, -1 for the _deprecated split
The thinking here was to move the deprecated functions out
of the way (out of sight, out of mind) for cases where we want
to consider the whole modern API.
The guestfs(3) man page already hides deprecated APIs IIRC.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v