Magnus Hagander wrote:
> Taking a look at the actual code here, and a few small nitpicks:
>
> + errdetail("Failed system call was %s,
> error code %lu", "LookupPrivilegeValue", GetLastError())));
>
> this seems to be a new pattern of code -- for other similar cases it just
> writes LookupPrivilegeValue inside the patch itself. I'm guessing the idea
> was for translatability, but I think it's better we stick to the existing
> pattern.
There are two reasons for doing things this way. One is that you reduce
the chances of a translator making a mistake with the function name (say
just a typo, or in egregious cases they may even translate the function
name). The other is that if you have many of these messages, you only
translate the generic part once instead of having the same message
a handful of times, exactly identical but for the function name.
So please do apply that kind of pattern wherever possible. We already
have the proposed error message, twice. No need for two more
occurrences of it.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
