> On 22 Apr 2025, at 01:19, Jacob Champion <jacob.champ...@enterprisedb.com>
> wrote:
> v8 also makes the following changes:
Thanks for this version, a few small comments:
+ if oauth_flow_supported
+ cdata.set('USE_LIBCURL', 1)
+ elif libcurlopt.enabled()
+ error('client OAuth is not supported on this platform')
+ endif
We already know that libcurlopt.enabled() is true here so maybe just doing
if-else-endif would make it more readable and save readers thinking it might
have changed? Also, "client OAuth" reads a bit strange, how about "client-side
OAuth" or "OAuth flow module"?
- appendPQExpBufferStr(&conn->errorMessage,
- libpq_gettext(actx->errctx));
- appendPQExpBufferStr(&conn->errorMessage, ": ");
+ appendPQExpBufferStr(errbuf, libpq_gettext(actx->errctx));
+ appendPQExpBufferStr(errbuf, ": ");
I think we should take this opportunity to turn this into a appendPQExpBuffer()
with a format string instead of two calls.
+ len = errbuf->len;
+ if (len >= 2 && errbuf->data[len - 2] == '\n')
Now that the actual variable, errbuf->len, is short and very descriptive I
wonder if we shouldn't just use this as it makes the code even clearer IMO.
--
Daniel Gustafsson
