On Wed, May 19, 2021 at 3:08 PM Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> wrote: > > On Wed, May 19, 2021 at 2:33 PM Amul Sul <sula...@gmail.com> wrote: > > > > On Wed, May 19, 2021 at 2:09 PM Bharath Rupireddy > > <bharath.rupireddyforpostg...@gmail.com> wrote: > > > > > > Hi, > > > > > > parse_subscription_options function has some similar code when > > > throwing errors [with the only difference in the option]. I feel we > > > could just use a variable for the option and use it in the error.
I am not sure how much it helps to just refactor this part of the code alone unless we need to add/change it more. Having said that, this function is being modified by one of the proposed patches for logical decoding of 2PC and I noticed that the proposed patch is adding more parameters to this function which already takes 14 input parameters, so I suggested refactoring it. See comment 11 in email[1]. See, if that makes sense to you then we can refactor this function such that it can be enhanced easily by future patches. > > > While this has no benefit at all, it saves some LOC and makes the code > > > look better with lesser ereport(ERROR statements. PSA patch. > > > > > > Thoughts? > > > > I don't have a strong opinion on this, but the patch should add > > __translator__ help comment for the error msg. > > Is the "/*- translator:" help comment something visible to the user or > some other tool? > We use similar comments at other places. So, it makes sense to retain the comment as it might help translation tools. [1] - https://www.postgresql.org/message-id/CAA4eK1Jz64rwLyB6H7Z_SmEDouJ41KN42%3DVkVFp6JTpafJFG8Q%40mail.gmail.com -- With Regards, Amit Kapila.