remove useless comments
The comments considering checking share/ directory was there for
almost 13 years, yet nobody ever trying to add the checking, and
there seems never any trouble for not checking it, then I think
we should remove those comments.
diff --git a/src/backend/postmaster/postmaster.c
b/src/backend/postmaster/postmaster.c
index 81cb585891..ecdc59ce5e 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1581,11 +1581,6 @@ getInstallationPaths(const char *argv0)
errhint("This may indicate an
incomplete PostgreSQL installation, or that the file \"%s\" has been
moved away from its proper location.",
my_exec_path)));
FreeDir(pdir);
-
- /*
- * XXX is it worth similarly checking the share/ directory? If the lib/
- * directory is there, then share/ probably is too.
- */
}--
Regards
Junwang Zhao
Attachments:
0001-comments-remove-useless-comments.patchapplication/octet-stream; name=0001-comments-remove-useless-comments.patchDownload+0-6
Junwang Zhao <zhjwpku@gmail.com> writes:
The comments considering checking share/ directory was there for
almost 13 years, yet nobody ever trying to add the checking, and
there seems never any trouble for not checking it, then I think
we should remove those comments.
I think that comment is valuable. It shows that checking the
sibling directories was considered and didn't seem worthwhile.
Perhaps it should be rephrased in a more positive way (without XXX),
but merely deleting it is a net negative because future hackers
would have to reconstruct that reasoning.
BTW, we're working in a 30+-year-old code base, so the mere fact
that a comment has been there a long time does not make it bad.
regards, tom lane
Fair enough, the rephased version of the comments is in the attachment,
please take a look.
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1583,8 +1583,8 @@ getInstallationPaths(const char *argv0)
FreeDir(pdir); /*
- * XXX is it worth similarly checking the share/ directory? If the lib/
- * directory is there, then share/ probably is too.
+ * It's not worth checking the share/ directory. If the lib/ directory
+ * is there, then share/ probably is too.
*/
}On Tue, Aug 9, 2022 at 11:50 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Junwang Zhao <zhjwpku@gmail.com> writes:
The comments considering checking share/ directory was there for
almost 13 years, yet nobody ever trying to add the checking, and
there seems never any trouble for not checking it, then I think
we should remove those comments.I think that comment is valuable. It shows that checking the
sibling directories was considered and didn't seem worthwhile.
Perhaps it should be rephrased in a more positive way (without XXX),
but merely deleting it is a net negative because future hackers
would have to reconstruct that reasoning.BTW, we're working in a 30+-year-old code base, so the mere fact
that a comment has been there a long time does not make it bad.regards, tom lane
--
Regards
Junwang Zhao
Attachments:
0001-comments-rephrase-the-comment-in-a-more-positive-way.patchapplication/octet-stream; name=0001-comments-rephrase-the-comment-in-a-more-positive-way.patchDownload+2-3
On Wed, Aug 10, 2022 at 12:24:02AM +0800, Junwang Zhao wrote:
Fair enough, the rephased version of the comments is in the attachment,
please take a look.--- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -1583,8 +1583,8 @@ getInstallationPaths(const char *argv0) FreeDir(pdir);/* - * XXX is it worth similarly checking the share/ directory? If the lib/ - * directory is there, then share/ probably is too. + * It's not worth checking the share/ directory. If the lib/ directory + * is there, then share/ probably is too. */ }
Patch applied to master.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.