Hello Justin,
Review about v34, up from v32 which I reviewed in January. v34 is an
updated version of v32, without the part about lstat at the end of the
series.
All 7 patches apply cleanly.
# part 01
One liner doc improvement about the directory flag.
OK.
# part 02
Add tests for various options on pg_ls_dir, and for pg_stat_file, which were not
exercised before. "make check" is ok.
OK.
# part 03
This patch adds a new pg_ls_dir_metadata. Internally, this is an extension of
pg_ls_dir_files function which is used by other pg_ls functions. Doc ok.
New tests are added which check that the result columns are configured as required,
including error cases.
"make check" is ok.
OK.
# part 04
Add a new "isdir" column to "pg_ls_tmpdir" output. This is a small behavioral
change.
I'm ok with that, however I must say that I'm still unsure why we would
not jump directly to a "type" char column. What is wrong with outputing
'd' or '-' instead of true or false? This way, the interface needs not
change if "lstat" is used later? ISTM that the interface issue should be
somehow independent of the implementation issue, and we should choose
directly the right/best interface?
Independently, the documentation may be clearer about what happens to
"isdir" when the file is a link? It may say that the behavior may change
in the future?
About homogeneity, I note that some people may be against adding "isdir"
to other ls functions. I must say that I cannot see a good argument not to
do it, and that I hate dealing with systems which are not homogeneous
because it creates surprises and some loss of time.
"make check" ok.
OK.
# part 05
Add isdir to other ls functions. Doc is updated.
Same as above, I'd prefer a char instead of a bool, as it is more extendable and
future-proof.
OK.
# part 06
This part extends and adds a test for pg_ls_logdir.
"make check" is ok.
OK.
# part 07
This part extends pg_stat_file with more date informations.
"make check" is ok.
OK.
# doc
Overall doc generation is OK.
--
Fabien.
