Re: Little cleanup of ShmemInit function names

On 28/08/2024 18:26, Andreas Karlsson wrote:
> On 8/7/24 2:08 PM, Heikki Linnakangas wrote:
>> The attached patches rename them to follow the usual naming convention.
>> The InitLocks() function is refactored a bit more, splitting the
>> per-backend initialization to a separate InitLockManagerAccess()
>> function. That's why it's in a separate commit.
>
> I like the idea behind the patches. And they still apply and build.
>
> The first patch is clean and well commented. I just have two minor nitpicks.
>
> Small typo with the extra "which" which makes the sentence not flow
> correctly
>
> "This is called from CreateSharedMemoryAndSemaphores(), which see for
> more comments."

Hmm, I don't see the issue. It's an uncommon sentence structure, but it
was there before this patch, and it's correct AFAICS. If you grep for
"which see ", you'll find some more examples of that.

> On the topic of minor language issues I think the comma below is redundant.
>
> "In the normal postmaster case, the shared hash tables are created here."

On second thoughts, I rearranged the sentences in the paragraph a
little, see attached.

> The second patch is a simple renaming which reduces mental load by
> making the naming more consistent so I like it. Also since these
> functions are not really useful for any extension authors I do not see
> any harm in renaming them.
>
> After cleaning up the language of that comment I think these patches can
> be committed.

Thanks for the review!

--
Heikki Linnakangas
Neon (https://neon.tech)