|From:||Michael Paquier <michael(at)paquier(dot)xyz>|
|To:||Tatsuro Yamada <yamada(dot)tatsuro(at)lab(dot)ntt(dot)co(dot)jp>|
|Cc:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, pgsql-hackers(at)postgresql(dot)org|
|Subject:||Re: Fix help option of contrib/oid2name|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
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
- 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
- Some formatting issues with option docs, leading to some noise diffs.
- Some typos in documentation.
- Documentation format was rather weird for some options, so I made the
- 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
|Next Message||Daniel Gustafsson||2018-08-27 10:06:18||Re: [HACKERS] Optional message to user when terminating/cancelling backend|
|Previous Message||Yugo Nagata||2018-08-27 09:34:17||Re: Refactor textToQualifiedNameList()|