From: | Joe Nelson <joe(at)begriffs(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Alvaro Herrera from 2ndQuadrant <alvherre(at)alvh(dot)no-ip(dot)org>, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Surafel Temesgen <surafel3000(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Change atoi to strtol in same place |
Date: | 2019-09-11 06:22:12 |
Message-ID: | 20190911062212.GA9113@begriffs.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Michael Paquier wrote:
> Using a wrapper in src/fe_utils makes sense. I would have used
> options.c for the new file, or would that be too much generic?
Sure, options.c sounds fine -- it doesn't seem any more generic than
"arg_utils" and is a little simpler. The only other question I have is
if the function inside -- with some adjustment in its interface -- might
be useful in other contexts beyond front-end args checking.
> The code indentation is weird, particularly the switch/case in
> pg_strtoint64_range().
I did run pgindent... Do I need to tell it about the existence of the
new file?
> The error handling is awkward.
Let's continue this point in your follow-up
<20190911035356(dot)GE1953(at)paquier(dot)xyz>.
> Why do you need to update common/Makefile?
Thought I needed to do this so that other parts would link properly, but
just removed the changes there and stuff still links OK, so I'll remove
that change.
> The builds on Windows are broken. You are adding one file into
> fe_utils, but forgot to update @pgfeutilsfiles in Mkvcbuild.pm. --
Thanks for the tip, I'm still learning about the build process. Is there
a service I can use to test my patches across multiple platforms? I'd
rather not bother reviewers with build problems that I can catch in a
more automated way.
--
Joe Nelson https://begriffs.com
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2019-09-11 06:25:59 | Re: psql - improve test coverage from 41% to 88% |
Previous Message | Pavel Stehule | 2019-09-11 06:14:21 | don't see materialized views in information_schema |