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.ge1...@paquier.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
