On Fri, 31 Jan 2025 at 21:05, Daniel Gustafsson <daniel@yesql.se> wrote:
>
> > On 31 Jan 2025, at 16:29, Andres Freund <andres@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
