On Wed, 7 Apr 2021 12:51:55 -0400
Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> On 2021-Apr-07, Jehan-Guillaume de Rorthais wrote:
>
> > When I'm creating a new node, I'm using the "pgaTester" factory class. It
> > relies on PATH to check the major version using pg_config, then loads the
> > appropriate class.
>
> From a code cleanliness point of view, I agree that having separate
> classes for each version is neater than what you call a forest of
> conditionals. I'm not sure I like the way you instantiate the classes
> in pgaTester though -- wouldn't it be saner to have PostgresNode::new
> itself be in charge of deciding which class to bless the object as?
> Since we're talking about modifying PostgresNode itself in order to
> support this, it would make sense to do that.
Yes, it would be much saner to make PostgresNode the factory class. Plus, some
more logic could be injected there to either auto-detect the version (current
behavior) or eg. use a given path to the binaries as Mark did in its patch.
> (I understand that one of your decisions was to avoid modifying
> PostgresNode, so that you could ingest whatever came out of PGDG without
> having to patch it each time.)
Indeed, that's why I created this class with a not-very-fortunate name :)
Let me know if it worth that I work on an official patch.
Regards,
