From: Fabien COELHO <coelho@cri.ensmp.fr>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: David Steele <david@pgmasters.net>
Cc: pgsql-hackers@postgresql.org, "Bossart, Nathan" <bossartn@amazon.com>
Cc: Thomas Munro <thomas.munro@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Subject: Re: pg_ls_tmpdir to show directories and shared filesets (and pg_ls_*)
Date: Sun, 12 Apr 2020 13:53:40 +0200 (CEST)
Message-ID: <alpine.DEB.2.21.2004121145190.5257@pseudo> (raw)
In-Reply-To: <20200331200812.GI14618@telsasoft.com>
References: <20200316214806.GQ26184@telsasoft.com>
<20200316221736.GA15635@alvherre.pgsql>
<20200317031427.GU26184@telsasoft.com>
<alpine.DEB.2.21.2003170828250.17584@pseudo>
<20200317190401.GY26184@telsasoft.com>
<20200331200812.GI14618@telsasoft.com>
Hello Justin,
About v15, seen as one patch.
Patches serie applies cleanly, compiles, "make check" ok.
Documentation:
- indent documentation text around 80 cols, as done around?
- indent SQL example for readability and capitalize keywords
(pg_ls_dir_metadata)
- "For each file in a directory, list the file and its metadata."
maybe: "List files and their metadata in a directory"?
Code:
- Most pg_ls_*dir* functions call pg_ls_dir_files(), which looks like
reasonable refactoring, ISTM that the code is actually smaller.
- please follow pg style, eg not "} else {"
- there is a "XXX" (meaning fixme?) tag remaining in a comment.
- file types: why not do block & character devices, fifo and socket
as well, before the unkown case?
- I'm wondering whether could pg_stat_file call pg_ls_dir_files without
too much effort? ISTM that the output structure nearly the same. I do
not like much having one function specialized for files and one for
directories.
Tests:
- good, there are some!
- indent SQL code, eg by starting a new line on new clauses?
- put comments on separate lines (I'm not against it on principle, I do
that, but I do not think that it is done much in test files).
--
Fabien.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: coelho@cri.ensmp.fr, pryzby@telsasoft.com, alvherre@2ndquadrant.com, david@pgmasters.net, bossartn@amazon.com, thomas.munro@gmail.com, tgl@sss.pgh.pa.us
Subject: Re: pg_ls_tmpdir to show directories and shared filesets (and pg_ls_*)
In-Reply-To: <alpine.DEB.2.21.2004121145190.5257@pseudo>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox