On 9/8/17 1:32 PM, Peter Eisentraut wrote: > > Yes, some of the error messages had changed. Fixed patches attached.
Patches apply and all tests pass. A few comments: * [PATCH v2 1/7] adminpack: Add test suite There are no regular tests for pg_logdir_ls(). It looks like TAP tests would be required but I'm not sure it's worth it. The fact that the "default" log name format is hard-coded in is, um, interesting. Maybe add: + SELECT pg_logdir_ls(); + ERROR: could not read directory "log": No such file or directory to get a little more coverage? It would be good to at least have a note on why it is not tested. * [PATCH v2 4/7] chkpass: Add test suite Well, this is kind of scary: +CREATE EXTENSION chkpass; +WARNING: type input function chkpass_in should not be volatile I guess the only side effect is that this column cannot be indexed? The docs say that, so OK, but is there anything else a user should be worried about? The rest looks good. I'll mark this "Ready for Committer" since I'm the only reviewer. I don't think anything you might add based on my comments above requires a re-review. As for testing on more platforms, send it to the build farm? -- -David da...@pgmasters.net -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
