Horton

Thread

Commits

  1. Add 'tap_tests' flag in config_default.pl

  1. Equivalent of --enable-tap-tests in MSVC scripts

    Michael Paquier <michael.paquier@gmail.com> — 2016-03-01T13:00:23Z

    Hi all,

As of now the MSVC scripts control if TAP tests are enabled or not
using a boolean flag as $config->{tap_tests}. However, this flag is
just taken into account in vcregress.pl, with the following issues:
1) config_default.pl does not list tap_tests, so it is unclear to
users to enable them. People need to look into vcregress.pl as a start
point.
2) GetFakeConfigure() does not translate $config->{tap_tests} into
--enable-tap-tests, leading to pg_config not reporting it in
CONFIGURE. This is inconsistent with what is done in ./configure.

Attached is a patch to address those two issues.
Regards,
-- 
Michael

  2. Re: Equivalent of --enable-tap-tests in MSVC scripts

    Craig Ringer <craig@2ndquadrant.com> — 2016-03-01T14:25:01Z

    On 1 March 2016 at 21:00, Michael Paquier <michael.paquier@gmail.com> wrote:

> Hi all,
>
> As of now the MSVC scripts control if TAP tests are enabled or not
> using a boolean flag as $config->{tap_tests}. However, this flag is
> just taken into account in vcregress.pl, with the following issues:
> 1) config_default.pl does not list tap_tests, so it is unclear to
> users to enable them. People need to look into vcregress.pl as a start
> point.
>

Seems like a no-brainer.


> 2) GetFakeConfigure() does not translate $config->{tap_tests} into
> --enable-tap-tests, leading to pg_config not reporting it in
> CONFIGURE. This is inconsistent with what is done in ./configure.
>

You seem to have re-indented config_default.pl at the same time.

That's worth doing but shouldn't be in the same patch, it makes it hard to
see what actually changed. Yes, that's a nitpick, but it makes a 1-liner
patch into a lot more.

-- 
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

  3. Re: Equivalent of --enable-tap-tests in MSVC scripts

    Andrew Dunstan <andrew@dunslane.net> — 2016-03-01T15:40:12Z

    
On 03/01/2016 08:00 AM, Michael Paquier wrote:
> Hi all,
>
> As of now the MSVC scripts control if TAP tests are enabled or not
> using a boolean flag as $config->{tap_tests}. However, this flag is
> just taken into account in vcregress.pl, with the following issues:
> 1) config_default.pl does not list tap_tests, so it is unclear to
> users to enable them. People need to look into vcregress.pl as a start
> point.
> 2) GetFakeConfigure() does not translate $config->{tap_tests} into
> --enable-tap-tests, leading to pg_config not reporting it in
> CONFIGURE. This is inconsistent with what is done in ./configure.
>
> Attached is a patch to address those two issues.
>

Good work. There seem to be some unrelated whitespace changes. Shouldn't 
this just be two extra lines?

cheers

andrew

  4. Re: Equivalent of --enable-tap-tests in MSVC scripts

    Michael Paquier <michael.paquier@gmail.com> — 2016-03-02T02:19:02Z

    On Wed, Mar 2, 2016 at 12:40 AM, Andrew Dunstan <andrew@dunslane.net> wrote:
> On 03/01/2016 08:00 AM, Michael Paquier wrote:
>> As of now the MSVC scripts control if TAP tests are enabled or not
>> using a boolean flag as $config->{tap_tests}. However, this flag is
>> just taken into account in vcregress.pl, with the following issues:
>> 1) config_default.pl does not list tap_tests, so it is unclear to
>> users to enable them. People need to look into vcregress.pl as a start
>> point.
>> 2) GetFakeConfigure() does not translate $config->{tap_tests} into
>> --enable-tap-tests, leading to pg_config not reporting it in
>> CONFIGURE. This is inconsistent with what is done in ./configure.
>>
>> Attached is a patch to address those two issues.
>
> Good work. There seem to be some unrelated whitespace changes. Shouldn't
> this just be two extra lines?

pertidy is telling me the contrary. Is that bad to run it for such a
change? I thought we cared about the format of the perl code, and the
length of the variable name "tap_tests" has an impact on the
indentation of the whole block per the rules in
src/tools/pgindent/perltidyrc. (Actually I made a mistake in previous
patch the portion for asserts should not be indented, not sure why it
was, attached is an updated patch).
-- 
Michael

  5. Re: Equivalent of --enable-tap-tests in MSVC scripts

    Craig Ringer <craig@2ndquadrant.com> — 2016-03-04T12:50:52Z

    On 2 March 2016 at 10:19, Michael Paquier <michael.paquier@gmail.com> wrote:

> On Wed, Mar 2, 2016 at 12:40 AM, Andrew Dunstan <andrew@dunslane.net>
> wrote:
> > On 03/01/2016 08:00 AM, Michael Paquier wrote:
> >> As of now the MSVC scripts control if TAP tests are enabled or not
> >> using a boolean flag as $config->{tap_tests}. However, this flag is
> >> just taken into account in vcregress.pl, with the following issues:
> >> 1) config_default.pl does not list tap_tests, so it is unclear to
> >> users to enable them. People need to look into vcregress.pl as a start
> >> point.
> >> 2) GetFakeConfigure() does not translate $config->{tap_tests} into
> >> --enable-tap-tests, leading to pg_config not reporting it in
> >> CONFIGURE. This is inconsistent with what is done in ./configure.
> >>
> >> Attached is a patch to address those two issues.
> >
> > Good work. There seem to be some unrelated whitespace changes. Shouldn't
> > this just be two extra lines?
>
> pertidy is telling me the contrary. Is that bad to run it for such a
> change? I thought we cared about the format of the perl code, and the
> length of the variable name "tap_tests" has an impact on the
> indentation of the whole block per the rules in
> src/tools/pgindent/perltidyrc. (Actually I made a mistake in previous
> patch the portion for asserts should not be indented, not sure why it
> was, attached is an updated patch).
>
>
If it's the result of perltidy changing its mind about the formatting as a
result of this change I guess we have to eyeroll and live with it. perltidy
leaves the file alone as it is in the tree currently, so that be it.

Gripe withdrawn, ready for committer IMO

-- 
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

  6. Re: Equivalent of --enable-tap-tests in MSVC scripts

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2016-03-04T16:10:21Z

    Craig Ringer wrote:

> If it's the result of perltidy changing its mind about the formatting as a
> result of this change I guess we have to eyeroll and live with it. perltidy
> leaves the file alone as it is in the tree currently, so that be it.
> 
> Gripe withdrawn, ready for committer IMO

Okay, thanks.  I applied it back to 9.4, which is when
--enable-tap-tests appeared.  I didn't perltidy 9.4's config_default.pl,
though.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  7. Re: Equivalent of --enable-tap-tests in MSVC scripts

    Craig Ringer <craig@2ndquadrant.com> — 2016-03-04T16:16:16Z

    On 5 March 2016 at 00:10, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

> Craig Ringer wrote:
>
> > If it's the result of perltidy changing its mind about the formatting as
> a
> > result of this change I guess we have to eyeroll and live with it.
> perltidy
> > leaves the file alone as it is in the tree currently, so that be it.
> >
> > Gripe withdrawn, ready for committer IMO
>
> Okay, thanks.  I applied it back to 9.4, which is when
> --enable-tap-tests appeared.  I didn't perltidy 9.4's config_default.pl,
> though.


Thanks very much. It didn't occur to me to backport it, but it seems
harmless.

https://commitfest.postgresql.org/9/566/ marked as committed.

-- 
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

  8. Re: Equivalent of --enable-tap-tests in MSVC scripts

    Michael Paquier <michael.paquier@gmail.com> — 2016-03-04T22:21:57Z

    On Sat, Mar 5, 2016 at 1:16 AM, Craig Ringer <craig@2ndquadrant.com> wrote:
> On 5 March 2016 at 00:10, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>>
>> Craig Ringer wrote:
>>
>> > If it's the result of perltidy changing its mind about the formatting as
>> > a
>> > result of this change I guess we have to eyeroll and live with it.
>> > perltidy
>> > leaves the file alone as it is in the tree currently, so that be it.
>> >
>> > Gripe withdrawn, ready for committer IMO
>>
>> Okay, thanks.  I applied it back to 9.4, which is when
>> --enable-tap-tests appeared.  I didn't perltidy 9.4's config_default.pl,
>> though.
>
>
> Thanks very much. It didn't occur to me to backport it, but it seems
> harmless.
>
> https://commitfest.postgresql.org/9/566/ marked as committed.

Thanks!
-- 
Michael