From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | Ashutosh Bapat <ashutosh(dot)bapat(at)2ndquadrant(dot)com> |
Cc: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Enumize logical replication message actions |
Date: | 2020-10-30 12:08:08 |
Message-ID: | CAA4eK1K+SPRXKNsjyUjjP25WE-Sx5PkxM3VunNxS4T=TTReDEA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Oct 30, 2020 at 5:05 PM Ashutosh Bapat
<ashutosh(dot)bapat(at)2ndquadrant(dot)com> wrote:
>
>
>
> On Fri, 30 Oct 2020 at 09:16, Amit Kapila <amit(dot)kapila16(at)gmail(dot)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.
From | Date | Subject | |
---|---|---|---|
Next Message | Ashutosh Bapat | 2020-10-30 12:22:00 | Re: Enumize logical replication message actions |
Previous Message | Ashutosh Bapat | 2020-10-30 11:36:57 | Re: Enumize logical replication message actions |