Hi Mark,

Thanks for starting the review.

On Mon, 2 Jul 2018 10:55:01 -0400
Mark Michelson <[email protected]> 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,
though.

Thanks,
Jakub
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to