On Wed, Mar 25, 2020 at 10:08:25AM +0100, Christoph Zwerschke wrote:
> Am 25.03.2020 um 02:20 schrieb Justin Pryzby:
> > The old stuff can be removed eventually.  I like this:
> >
> >          if self.escaping_funcs and pg_version >= (9, 0):
> >              define_macros.append(('ESCAPING_FUNCS', None))
> > That allows a packager to disable a feature
> > if they want the abilityto run> against old libpq.
> The problem is that these switches are by default set to true. The idea was
> that you set them to false on the command line if you don't want these
> features to be compiled. However, I think that never worked since you simply
> can't set these flags to false on the command line, you only can set them to
> true by specifying the names of the options (i.e. pass --escaping-funcs as
> option).
> So we need to change that and instead use the negatives of these flags
> everywhere as options i.e. "no_escaping_funcs" in this case.

I think I would leave the "positive" variable names, and *add* negative forms
of the commandline options.  --no-foo and/or --foo=no

Also, I believe it used to be a best-practice for distribution packagers to
specify --with-foo --with-bar --with-baz even if those were the defaults (or if
they defaulted to true if libfoo/bar/baz were installed).  That avoids silently
building packages with reduced functionality if libfoo isn't found at compile
time.  So logic like this might be nice:

if self.enable_memsize and pg_version < (12, 0):
        error('memsize not supported until pg12')

postgres vacuum options have a ternary system: on/off/default.  If you need
more than a line of logic, maybe you'd write:

if self.enable_memsize is True and pg_version < (12, 0):
        error('memsize not supported until pg12')
else if self.enable_memsize == -1: # default/unset
        self.enable_memsize = bool(pg_version >= (12, 0):)
if self.enable_memsize is True:
        define_macros.append(('WITH_MEMSIZE', None))
PyGreSQL mailing list

Reply via email to