Hello, I had a look on this patch.
Let me show you some comments about the README, Makefile and
buffer_capture_cmp of the second part for the present. A
continuation of this comment would be seen later..
- contrib/buffer_capture_cmp/README
- 'contains' seems duplicate in the first paragraph.
- The second pragraph says that 'This script can use the node
number of the master node available as the first argument of
the script when it is run within the test suite.' But test.sh
seems not giving such a parameter.
- contrib/buffer_capture_cmp/Makefile
"make check" does nothing when BUFFER_CAPTURE is not defined, as
described in itself. But I trapped by that after build the
server by 'make CFLAGS="-DBUFFER_CAPTURE"':( It would be better
that 'make check' without defining it prints some message.
- buffer_capture_cmp.c
This source generates void executable when BUFFER_CAPTURE is
not defined. The restriction seems to be put only to use
BUFFER_CAPTURE_FILE in bufcapt.h. If so, changing the parameter
of the executable as described in the following comment for
main() would blow off the necessity for the restriction.
- buffer_capture_cmp.c/main()
The parameters for this command are the parent directories for
each capture file. This is a bit incovenient for separate
use. For example, when I want to gather the capture files from
multiple servers then compare them, I should unwillingly make
their own directories for each capture file. If no particular
reason exists for the design, I suppose it would be more
convenient that the parameters are the names of the capture
files themselves.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
