В письме от 24 апреля 2017 09:01:18 пользователь Fabien COELHO написал:
> > To sum up:
> >
> > - I agree to add a generic command TestLib & a wrapper in PostgresNode,
> >
> > instead of having pgbench specific things in the later, then call
> > them from pgbench test script.
> >
> > - I still think that moving the pgbench scripts inside the test script
> >
> > is a bad idea (tm).
My sum up is the following:
I see my job as a reviewer is to tell "I've read the code, and I am sure it is
good".
I can tell this about this code, if:
- There is no pgbench specific staff in src/test/perl. Or there should be
_really_big_ reason for it.
- All the testing is done via calls of TestLib.pm functions. May be these
functions should be improved somehow. May be there should be some warper
around them. But not direct IPC::Run::run call.
- All the pgbench scripts are stored in one file. 36 files are not acceptable.
I would include them in the test script itself. May be it can be done in other
ways. But not 36 less then 100 byte files in source code tree...
May be I am wrong. I am not a guru. But then somebody else should say "I've
read the code, and I am sure it is good" instead of me. And it would be his
responsibility then. But if you ask me, issues listed above are very
important, and until they are solved I can not tell "the code is good", and I
see no way to persuade me. May be just ask somebody else...
--
Nikolay Shaplov, independent Perl & C/C++ developer. Available for hire.
