From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
---|---|
To: | Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: postgres_fdw: Obsolete comments in GetConnection() |
Date: | 2021-10-06 09:36:56 |
Message-ID: | CALj2ACXrrqi8R_iFq0-70q6LUfEjM9XipsO2MDRu1oH4-Dj+XQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Oct 6, 2021 at 2:35 PM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> wrote:
>
> While working on something else, I noticed $SUBJECT, which I should
> have updated in commit 27e1f1456. :-( There are two places that need
> to be updated, but in the first place the second one seemed a bit
> redundant to me, because it says the same thing as the first one, and
> is placed pretty close to the first one within 10 lines or so. So I
> rewrote the second one entirely into something much more simple like
> the attached.
+1 for rewording the comments. Here are my thoughts on the patch:
1) Just to be consistent(we are using this word in the error message,
and in other comments around there), how about
+ * Determine whether to try to reestablish the connection.
instead of
+ * Determine whether to try to remake the connection later.
2) Just to be consistent, how about
+ * cases where we're starting new transaction (not subtransaction),
if a broken connection is
instead of
+ * cases where we're out of all transactions, if a broken connection is
3) IMO we don't need the word "later" here because we are immediately
reestablishing the connection, if it is decided to do so.
+ * Determine whether to try to remake the connection later.
The word "later" here in the comment below makes sense but not in the
above comment.
+ * detected, we try to reestablish a new connection later.
Regards,
Bharath Rupireddy.
From | Date | Subject | |
---|---|---|---|
Next Message | Ashutosh Bapat | 2021-10-06 09:53:11 | sentPtr jumping back at the beginning of logical replication |
Previous Message | Magnus Holmgren | 2021-10-06 09:08:57 | Capitalization of localized month and day names (to_char() with 'TMmonth', 'TMday', etc.) |