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.


Kyotaro Horiguchi
NTT Open Source Software Center

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to