On Wed, Jan 7, 2026 at 3:33 PM albert tan <alterttan1223@gmail.com> wrote:
> Hi Join,
> Thanks for pointing out extra issues. I have addressed them in v2.
Thanks, but I can't use this -- it looks like you just picked the
first difference you saw in my proposed edits and ignored the rest,
leaving the result just as awkward or ungrammatical as before (see
below). That doesn't save me any time.
> >/*
> > * The directory parameter can be omitted, absolute, or relative to the
> > * installation's base directory, which can be the sharedir or a custom
> > * path that it was set extension_control_path. It depends where the
> > * .control file was found.
> > */
> >
> >I imagine this means "or a custom path that was set via extension_control_path".
- * path that it was set extension_control_path. It depends where the
+ * path that was set extension_control_path. It depends where the
And, while looking again, there's another error here: "depends on"
> >/*
> > * Return a list of directories declared on extension_control_path GUC.
> > */
>
> >I would guess this normally phrased something like "Return the list of
> >directories in extension_control_path".
- * Return a list of directories declared on extension_control_path GUC.
+ * Return the list of directories declared on extension_control_path GUC.
My main gripe here was "declared on <foo> GUC", not the article, which
on second thought may as well remain "a list" since we do use that
elsewhere to mean "an instance of the List type". I went with "Return
a list of directories declared in the extension_control_path GUC.".
Also, I decided that the paragraph that started this report needs to
be rewritten:
> ' The control file will be search on Extension_control_path paths if
> control->control_dir is NULL, otherwise it will use the value of control_dir
> to read and parse the .control file, so it assume that the control_dir is a
> valid path for the control file being parsed.'
- I'm not sure what the "assume" phrase is explaining, since we always
check for errors when opening a file. The contract elsewhere for
filling in control_dir doesn't seem as relevant as the NULL behavior.
- Passive voice is often used to maintain discourse cohesion, but the
following "it" doesn't refer to anything in particular.
On Fri, Jan 9, 2026 at 1:17 PM li carol <carol.li2025@outlook.com> wrote:
> This should be corrected to:
>
> "Works in a very similar way to find_in_path, but it receives an already parsed List of paths to search the basename,
andit does not support macro replacement or custom error messages (for simplicity). By 'already parsed List of paths'
thisfunction expects that the paths already have all macros replaced."
Thanks, but I see more that should be done here (also, for something
this long it's better to just mention what the corrections are, or
write a patch, so I don't have to figure it out). The original comment
is awkwardly written in that it has a separate paragraph to explain
what a term in the first paragraph means, so I simplified it to say
what it means.
This has now gone beyond a trivial fix, so I've attached what I
propose to commit barring other opinions.
--
John Naylor
Amazon Web Services
