On Wed, Nov 20, 2013 at 8:08 AM, Oskari Saarenmaa <os@ohmu.fi> wrote:
>
> On Mon, Nov 18, 2013 at 08:48:13PM -0500, Peter Eisentraut wrote:
> > On Tue, 2013-11-05 at 18:29 +0200, Oskari Saarenmaa wrote:
> > > This can be used to tag custom built packages with an extra version string
> > > such as the git describe id or distribution package release version.
> >
> > I think this is a reasonable feature, and the implementation is OK, but
> > I don't see why the format of the extra version information needs to be
> > exactly
> >
> > PG_VERSION="$PACKAGE_VERSION ($withval)"
> >
> > I'd imagine, for example, that someone will want to do -something or
> > +something. So I'd just make this
> >
> > PG_VERSION="$PACKAGE_VERSION$withval"
> >
> > Comments?
>
> Sounds reasonable.
>
> > > +# Allow adding a custom string to PG_VERSION
> > > +PGAC_ARG_REQ(with, extra-version, [NAME], [extra version information],
> > > +[PG_VERSION="$PACKAGE_VERSION ($withval)"], [PG_VERSION="$PACKAGE_VERSION"])
> >
> > This could be indented better. It was a bit confusing at first.
>
> Agreed. Attached an updated patch, or you can grab it from
> https://github.com/saaros/postgres/compare/extra-version
Here are a couple of comments about the patch:
1) I think that you should regenerate ./configure as well with this
patch to include all the changes together (someone correct me if I am
wrong here!)
2) This new option should be added in the section "## Command line
options" in configure.in
3) PG_VERSION is not a variable name adapted IMO, as it might contain
custom information. Something like PG_VERSION_TOTAL perhaps?
regards,
--
Michael
