On Sat, Nov 1, 2008 at 3:42 PM, Ron Mayer <[EMAIL PROTECTED]> wrote: > # Patch 3: "cleanup.patch" > Fix rounding inconsistencies and refactor interval input/output code >
Compile, testing and regression tests all checked out.
I've picked up on a few code style issues, fixes attached.
If I'm reading the patch correctly, it seems you've renamed two of the
functions in datetime.c:
* AdjustFractionalSeconds => AdjustFractSeconds
* AdjustFractionalDays => AdjustFractDays
To be frank, this doesn't really seem worthwhile. It only saves five
characters in the name. What was your reason for renaming them?
I *was* going to question the inconsistent use of a space between the
pointer qualifier and the argument name, for example:
static char *
AddVerboseIntPart(char * cp, int value, char *units,
bool * is_zero, bool *is_before)
But then I noticed that there's a lot of this going on in datetime.c,
some of it appears to predate your patches. So I guess cleaning this
up in your function definitions would be a bit of a bolted-horse,
barn-door affair. Unless you felt like cleaning it up throughout the
file, it's probably not worth worrying about.
There are some very large-scale changes to the regression tests. I'm
finding it difficult to grok the nature of the changes from reading a
diff. If possible, could you post some quick notes on the
purpose/rationale of these changes?
Cheers,
BJ
cleanup-1.diff
Description: Binary data
-- Sent via pgsql-hackers mailing list ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
