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 -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers