On Sat, Nov 1, 2008 at 3:42 PM, Ron Mayer <rm_pg@cheapcomplexdevices.com> 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