On Tue, Jan 21, 2020 at 07:49:34AM -0600, Justin Pryzby wrote:
> Rebased against 40d964ec997f64227bc0ff5e058dc4a5770a70a9
> I added to March CF https://commitfest.postgresql.org/27/2425/
Please be careful with the sets of patches sent to a thread, just to
say that what you are sending is organized in a messy way, and that
this is not the only thread (I can see sometimes the same patches sent
to multiple threads for no actual reason). First, patches 0001 and
0002 have nothing to do with this thread. Patches 0003 and 0005 could
just be merged together, visibly with 0004 as well as they treat of
the same concepts, actually related to this thread. My point is that
it is harder to understand what you are trying to do, and that this is
inconsistent with the threads created.
From patch 0002:
- * If the all-visible page is turned out to be all-frozen but not
+ * If the all-visible page turned out to be all-frozen but not
* marked, we should so mark it. Note that all_frozen is only valid
* if all_visible is true, so we must check both.
Shouldn't the last part of the sentence be "we should mark it so"
instead of "we should so mark it"? I would rephrase the whole as
follows:
"If the all-visible page is all-frozen but not marked as such yet,
mark it as all-frozen."
From patch 0003:
/*
+ * Indent multi-line DETAIL if being sent to client (verbose)
+ * We don't know if it's sent to the client (client_min_messages);
+ * Also, that affects output to the logfile, too; assume that it's more
+ * important to format messages requested by the client than to make
+ * verbose logs pretty when also sent to the logfile.
+ */
+ msgprefix = elevel==INFO ? "!\t" : "";
Such stuff gets a -1 from me. This is not project-like, and you make
the translation of those messages much harder than they should be.
--
Michael
