On Mon, Dec 09, 2019 at 05:11:10PM -0500, Tom Lane wrote:
> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
>> We have a not-consistently-used convention that names in CamelCase are
>> used for global variables. Naming a function parameter in that style
>> seems pointlessly confusing. I would rather use redorecptr as Tom
>> suggested, which fits with the style used for the other arguments of
>> that function. BTW prepending an X or a p looks like minimum effort...
>> I'd stay away from that.
>
> Actually, for the particular case of RemoveXlogFile(s), I wonder if it
> shouldn't be "startptr" to go with the other argument "endptr". This line
> of thinking might not lead to nicer names in other functions, of course.
> But we shouldn't assume that a one-size-fits-all solution is going to
> improve legibility, and in the end, legibility is what this should be
> about.
Hmm. In the case of this logic, we are referring to the current end
of WAL with endptr, and what you are calling the startptr is really
the redo LSN of the last checkpoint in all the routines which are now
confused with RedoRecPtr: RemoveOldXlogFile, RemoveXlogFile and
XLOGfileslop. Using lower-case for all the characters of the variable
name sounds like a good improvement as well, so taking a combination
of all that I would just use "lastredoptr" in those three code paths
(note that we used to have PriorRedoPtr before). As that's a
confusion I introduced with d9fadbf, I would like to fix that and
backpatch this change down to 11. (Ranier gets the authorship
per se as that's extracted from a larger patch).
--
Michael
