On Thu, Jun 4, 2026 at 6:32 PM Álvaro Herrera <[email protected]> wrote:
> I think it would be better if the comments mentioned the function name > where the operation in question takes place rather than the file name; > for instance > > @@ -6223,7 +6223,7 @@ xact_redo_commit(xl_xact_parsed_commit *parsed, > * If a transaction completion record arrives that has as-yet > * unobserved subtransactions then this will not have been > fully > * handled by the call to RecordKnownAssignedTransactionIds() > in the > - * main recovery loop in xlog.c. So we need to do bookkeeping > again to > + * main recovery loop in PerformWalRecovery(). So we need to > do bookkeeping again to > > and so on. I think this is more helpful for a reader because they have > a precise point to jump to instead of having to scroll through the file > looking for the place. Also this doesn't get outdated if the function > is moved to a different file (not that this happens very often.) > Yes I agree. > > > @@ -118,7 +118,7 @@ StartupProcShutdownHandler(SIGNAL_ARGS) > > /* > > * Re-read the config file. > > * > > - * If one of the critical walreceiver options has changed, flag xlog.c > > + * If one of the critical walreceiver options has changed, flag > > xlogrecovery.c > > * to restart it. > > */ > > static void > > This could be, maybe "If ... has changed, make > StartupRequestWalReceiverRestart() aware of that." or something like > that. I am not sure whether we should be mentioning the function name which is just being called a few lines below. So maybe it's better to change the wording here. * - * If one of the critical walreceiver options has changed, flag xlog.c - * to restart it. + * If one of the critical walreceiver options has changed, request the startup + * process to restart the walreceiver. */ Let me know if this sounds ok. I am attaching the new version. Thanks Imran Zaheer
v2-0001-Update-comments-to-refer-to-the-correct-functions.patch
Description: Binary data
