On 2021-Mar-24, Andrew Dunstan wrote:
> OK, like this?
Yeah, looks good!
> +# Private routine to return a copy of the environment with the PATH and (DY)LD_LIBRARY_PATH
> +# correctly set when there is an install path set for the node.
> +# Routines that call Postgres binaries need to call this routine like this:
> +#
> +# local %ENV = $self->_get_install_env{[%extra_settings]);
> +#
> +# A copy of the environmnt is taken and node's host and port settings are added
> +# as PGHOST and PGPORT, Then the extra settings (if any) are applied. Any setting
> +# in %extra_settings with a value that is undefined is deleted; the remainder are
> +# set. Then the PATH and (DY)LD_LIBRARY_PATH are adjusted if the node's install path
> +# is set, and the copy environment is returned.
There's a typo "environmnt" here, and a couple of lines appear
overlength.
> +sub _get_install_env
I'd use a name that doesn't have "install" in it -- maybe _get_env or
_get_postgres_env or _get_PostgresNode_env -- but don't really care too
much about it.
> +# The install path set in get_new_node needs to be a directory containing
> +# bin and lib subdirectories as in a standard PostgreSQL installation, so this
> +# can't be used with installations where the bin and lib directories don't have
> +# a common parent directory.
I've never heard of an installation where that wasn't true. If there
was a need for that, seems like it'd be possible to set them with
{ bindir => ..., libdir => ...} but I doubt it'll ever be necessary.
--
Álvaro Herrera Valdivia, Chile
"We're here to devour each other alive" (Hobbes)
