Re: [PATCH 02/23] fsmonitor-ipc: create client routines for git-fsmonitor--daemon

On 4/26/2021 4:20 PM, Eric Sunshine wrote:

On Mon, Apr 26, 2021 at 10:31 AM Derrick Stolee [off-list ref] wrote:

quoted

On 4/1/21 11:40 AM, Jeff Hostetler via GitGitGadget wrote:

quoted

+#ifdef HAVE_FSMONITOR_DAEMON_BACKEND
+#define FSMONITOR_DAEMON_IS_SUPPORTED 1
+#else
+#define FSMONITOR_DAEMON_IS_SUPPORTED 0
+#endif
+
+int fsmonitor_ipc__is_supported(void)
+{
+     return FSMONITOR_DAEMON_IS_SUPPORTED;
+}

I don't see any other use of FSMONITOR_DAEMON_IS_SUPPORTED,
so I was thinking you could use the #ifdef/#else/#endif
construct within the implementation of this method instead
of creating a macro outside. But my suggestion might be an
anti-pattern, so feel free to ignore me.

On this project, it is preferred to keep the #if / #else / #endif
outside of functions since embedding them within functions often makes
it difficult to follow how the code flows (and generally makes
functions unnecessarily noisy). So, the way Jeff did this seems fine.

Makes sense.

An alternative would have been:

  #ifdef HAVE_FSMONITOR_DAEMON_BACKEND
  #define fsmonitor_ipc__is_supported() 1
  #else
  #define fsmonitor_ipc__is_supported() 0
  #endif

which would still allow calling it as a function:

    if (fsmonitor_ipc__is_supported())
        ...

but it's subjective whether that's actually any cleaner or better.

 
True. I'm just thinking about a future where we need to do a runtime
check for compatibility, but let's use the YAGNI principle and skip
it for now.

Thanks,
-Stolee