Re: [libvirt] [PATCH REPOST 20/38] virlog: Split output parsing and output defining to separate operations
On 05/04/2016 10:30 AM, Erik Skultety wrote:
Everything has been prepared to successfully split parsing and
defining logic
to separate operations.
---
src/util/virlog.c | 160 +++++++++++++++++++++++++++++------------------------
src/util/virlog.h | 15 +++--
tests/testutils.c | 19 ++++++-
tests/virlogtest.c | 5 +-
4 files changed, 114 insertions(+), 85 deletions(-)
Hmm. I see what you were going for with this patch layout: line up all the
pieces so you can internally convert everything in one fell swoop with patch
#20 and #21. However that makes it difficult to review IMO, most of the
critical change is bottled up in these few patches that depend on the previous
20 patches of context.
If instead you had (just an idea) renamed all the poorly named functions to
more descriptive names (like virLogParse* functions to virLogParseAndSet*) up
front, you could then split out virLogParse* and virLogSet* functions
completely internally to virlog.c and transparent to the current users, the
patches would be more individually self contained. Then you could convert all
the current users to the new functions, and drop the old poorly named ones.
I realize reworking that will be a pain so I'll try and review as is (more
tomorrow), but I'll jump around a bit. Also when posting the next round, I
suggest just splitting out the logging rework first, then once all of that is
committed, post a second series with the API bits.
- Cole