On Fri, Aug 24, 2018 at 01:32:47PM +0900, Tatsuro Yamada wrote:
> I revised the patch and created new tap tests.
Thanks, I have looked at the patch set. And here are some notes about
issues found while reviewing. For oid2name:
- Documentation had some typos, --tablename was used instead of --table.
- Using plural forms for some options makes more sense to me, like
--indexes, --system-objects, --tablespaces
- I have tweaked the description of -H in usage().
- An '=' was missing for --filenode
- I have reordered the options in alphabetical order.
- Added some basic tap tests, and added correct handling of the tests in
oid2name/Makefile.
- Inclusion of getopt_long.h has been missing, this would have caused
failures on Windows.
- getopt_long() has been reordered to be alphabetical. Same thing for
long_options[]. This has the advantage to ease the review and code
read.
- Some formatting issues with option docs, leading to some noise diffs.
For vacuumlo:
- Some typos in documentation.
- Documentation format was rather weird for some options, so I made the
whole consistent.
- I agree with the option names you put.
- Reorganization of the options.
- Added basic TAP tests, and fixed Makefile.
- default clause was missing from the option set.
Attached are updated patches, which I would be fine to commit. What do
you think?
--
Michael
