On Fri, Oct 30, 2020 at 5:05 PM Ashutosh Bapat <ashutosh.ba...@2ndquadrant.com> wrote: > > > > On Fri, 30 Oct 2020 at 09:16, Amit Kapila <amit.kapil...@gmail.com> wrote >> >> I think we can simply use 'return apply_handle_begin;' instead of >> adding return in another line. Again, I think we changed this handling >> in apply_dispatch() to improve the case where we can detect at the >> compile time any missing enum but at this stage it is not clear to me >> if that is true. > > > I don't see much value in writing it like "return apply_handle_begin()"; > gives an impression that apply_handle_begin() and apply_dispatch() are > returning something which they are not. I would prefer return on separate > line unless there's something more than style improvement. >
Fair enough. > I have added rationale behind Enum in the commit message as you suggested in > one of the later mails. > > PFA patch addressing your comments. > I don't like the word 'Enumize' in commit message. How about changing it to something like: (a) Add defines for logical replication protocol messages, or (b) Associate names with logical replication protocol messages. + 2. It's easy to locate the code handling a given type. In the above instead of 'type', shouldn't it be 'message'. Other than that the patch looks good to me. -- With Regards, Amit Kapila.
