On Fri, 31 Jan 2025 at 21:05, Daniel Gustafsson <dan...@yesql.se> wrote:
>
> > On 31 Jan 2025, at 16:29, Andres Freund <and...@anarazel.de> wrote:
>
> >> #ifdef ENABLE_GSS
> >> -#if defined(HAVE_GSSAPI_H)
> >> -#include <gssapi.h>
> >> -#else
> >> -#include <gssapi/gssapi.h>
> >> -#endif /* HAVE_GSSAPI_H */
> >> +#include "libpq/pg-gssapi.h"
> >> #endif /* ENABLE_GSS */
> >
> > This #ifdef ENABLE_GSS probably isn't necessary anymore.
>
> Yeah, I only left it for code documentation reasons to keep readers from
> thinking the ifdef was missing and had to go chase it in the new file.  It's
> definitely not required though I for sure don't mind removing it if others 
> feel
> it's pointless.

Few thoughts:
1) I also felt that this could be removed.

2) Was the copyright year retained as 1996 intentionally for the new
"pg-gssapi.h" file added because the contents were copied from other
files?
+ * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California

I see in few other places were new file was created, it was mentioned
as "Copyright (c) 2025, PostgreSQL Global Development Group"

3) Apart from that, there was a small whitespace issue while applying the patch:
git am v1-0001-Move-GSSAPI-includes-into-its-own-header.patch
Applying: Move GSSAPI includes into its own header
.git/rebase-apply/patch:116: new blank line at EOF.
+
warning: 1 line adds whitespace errors.

Overall patch looks good to me.

Regards,
Vignesh


Reply via email to