Re: Fix help option of contrib/oid2name

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
Date: 2018-08-27 10:03:18
Message-ID: 20180827100318.GD5553@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
0001-Rework-option-set-of-oid2name.patch text/x-diff 14.1 KB
0002-Rework-option-set-of-vacuumlo.patch text/x-diff 9.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
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()