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