[ovs-dev] [PATCH 01/23] db-ctl-base: Don't die in parse_column_names() on error.
jkbs at redhat.com
Tue Jul 3 08:41:27 UTC 2018
Thanks for starting the review.
On Mon, 2 Jul 2018 10:55:01 -0400
Mark Michelson <mmichels at redhat.com> wrote:
> Your cover letter still hasn't come through, so feel free to tell me if
> my comments here are misguided.
Looks like the cover letter has arrived now.
> I'm guessing that you did not remove ctl_fatal() calls from
> parse_commands() and ctl_parse_commands() because there is no
> ctl_context present. However, it's not clear why the ctl_fatal() call in
> cmd_add() was not converted to use ctl_error() + return.
I left ctl_parse_commands() & parse_commands() as is for now because
I'm not sure yet if I will end up using them for the ovn-nbctl daemon
mode in their current form.
cmd_add(), OTOH, is an oversight. Thanks for pointing it out. Let me
correct that in v2.
> I think patches 15 and 19 could be combined into one.
> You *could* combine patches 1-13 into one patch, 16-18 into one patch,
> and 20-23 in one patch, but I don't feel strongly enough about this to
> make you have to re-do the whole thing.
Squashing patches together is not a problem. It depends on what what we
agree on to be one logical change.
I find smaller patches easier to review without losing the context
midway through it. Not to the point when the patch appears disjointed,
More information about the dev