[ovs-dev] [PATCH 01/23] db-ctl-base: Don't die in parse_column_names() on error.

Jakub Sitnicki jkbs at redhat.com
Tue Jul 3 08:41:27 UTC 2018

Hi Mark,

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 mailing list