On Tue, Apr 25, 2017 at 05:15:24PM -0400, John Ferlan wrote:
v1:
https://www.redhat.com/archives/libvir-list/2017-April/msg01051.html
Changes since v1:
- Patches 1, 2, 4, and 14 were pushed since they were ACK and "separable"
- Former patch 3 is now patch 1
-> Restore the comments for the functions.
- Former patch 5 is now split into patch 2 and 3
-> Patch 2 addresses the "if (!obj)" check in virSecretObjListRemove
-> Patch 3 handles the naming of the variables
- Former patch 6 is now patch 4
-> Alterations here are not creating a @def for the express purpose
of only dereferencing obj->def if that's all that's used. If though
there is an obj->def->X, then the @def is created
- Former patch 7 is removed (e.g. keep virSecretObjListGetUUIDs where
it was). A battle for a different day perhaps.
- Former patch 8 is now patches 5 and 6
-> Patch 5 handles just the cleanup path for virSecretObjListGetUUIDs
-> Patch 6 changes the variable names
- Former patch 9 is now patches 7 and 8
-> Patch 7 focuses only on changing the name of @filter to @aclfilter
(something that wasn't called out specifically in the initial review
but that I figured I'd do anyway)
-> Patch 8 does the split and keeps the Callback functions together
so it doesn't appear that virSecretObjListNumOfSecrets moved. Again
I'll pick that battle another day. I did change the name of the
structures to include "virSecret" prefix though.
- Former patch 10 is now patch 9
-> This was ACK'd but not "easy" to extract out, so nothing to do
here.
- Former patch 11 is now patch 10
-> Perhaps merge conflicts were changed here. The algorithm hasn't
changed though.
- Former patch 12 is now patch 11
-> This was ACKd already - just forgot to extract it out for a push.
- Former patch 13 is removed
So I've tried to go with the spirit of the review even though I disagree
with the don't do the code motion part. I'll deal with that later when
things really converge and the functions are shorter.
ACK series if you fix the nits pointed out for patch 03 and 04 and if you
drop patch 10.
Pavel