Re: [HACKERS] Add Roman numeral conversion to to_number - Mailing list pgsql-hackers

From Douglas Doole
Subject Re: [HACKERS] Add Roman numeral conversion to to_number
Date
Msg-id CADE5jYK_-hNS2=jcHETTPbPEpGxSW6bAMPZeDN9oVTh-fn05xg@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] Add Roman numeral conversion to to_number  (Oliver Ford <ojford@gmail.com>)
Responses Re: [HACKERS] Add Roman numeral conversion to to_number  (Douglas Doole <dougdoole@gmail.com>)
List pgsql-hackers
Ah. Sorry I missed them - I'll give them a look. (Won't be able to get to it until Saturday though.)
On Thu, Sep 14, 2017 at 10:06 PM Oliver Ford <ojford@gmail.com> wrote:
I'll fix the brace, but there are two other patches in the first email for tests and docs. For some reason the commitfest app didn't pick them up.

On Friday, 15 September 2017, Doug Doole <DougDoole@gmail.com> wrote:
The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, passed
Spec compliant:           not tested
Documentation:            not tested

Code looks fine, but one niggly complaint at line 146 of the patch file ("while (*cp) {"). A K&R style brace slipped in, which doesn't match the formatting of the file.

Given that this is providing new formatting options, there should be new tests added that validate the options and error handling.

There's also the "do we want this?" debate from the discussion thread that still needs to be resolved. (I don't have an opinion either way.)

I'm sending this back to the author to address the first two issues.

The new status of this patch is: Waiting on Author

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

pgsql-hackers by date:

Previous
From: amul sul
Date:
Subject: Re: [HACKERS] [POC] hash partitioning
Next
From: Amit Langote
Date:
Subject: Re: [HACKERS] Race between SELECT and ALTER TABLE NO INHERIT