Hi Ævar,
On Tue, 13 Jul 2021, Ævar Arnfjörð Bjarmason wrote:
>
> On Tue, Jul 13 2021, Jeff Hostetler wrote:
>
> > On 7/1/21 7:02 PM, Ævar Arnfjörð Bjarmason wrote:
> >> On Thu, Jul 01 2021, Jeff Hostetler via GitGitGadget wrote:
> >>
> >>> From: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
> >>>
> >>> Teach the win32 backend to register a watch on the working tree
> >>> root directory (recursively). Also watch the <gitdir> if it is
> >>> not inside the working tree. And to collect path change notifications
> >>> into batches and publish.
> >>>
> >>> Signed-off-by: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
> >>> ---
> >>> compat/fsmonitor/fsmonitor-fs-listen-win32.c | 530 +++++++++++++++++++
> >> <bikeshed mode> Spying on the early history of this (looking for the
> >> Linux backend) I saw that at some point we had just
> >> compat/fsmonitor/linux.c, and presumably some of
> >> compat/fsmonitor/{windows,win32,macos,darwin}.c.
> >> At some point those filenames became much much longer.
> >>
> >
> > Once upon a time having "foo/bar/win32.c" and "abc/def/win32.c"
> > would cause confusion in the debugger (I've long since forgotten
> > which). Breaking at win32.c:30 was no longer unique.
> >
> > Also, if the Makefile sends all .o's to the root directory or a
> > unified OBJS directory rather than to the subdir containing the .c,
> > then we have another issue during linking...
> >
> > So, having been burned too many times, I prefer to make source
> > filenames unique when possible.
>
> A much shorter name like compat/fsmonitor/fsmon-win32.c would achieve
> that goal.
>
> >> I've noticed you tend to prefer really long file and function names,
> >> e.g. your borrowed daemonize() became
> >> spawn_background_fsmonitor_daemon(), I think aiming for shorter
> >> filenames & function names helps, e.g. these long names widen diffstats,
> >> and many people who hack on the code stick religiously to 80 character
> >> width terminals.
> >>
> >
> > I prefer self-documenting code.
>
> Sure, I'm not saying daemonize() is an ideal name, just suggesting that
> you can both get uniqueness & self-documentation and not need to split
> to multiple lines in some common cases to stay within the "We try to
> keep to at most 80 characters per line" in CodingGuidelines in this
> series.
While you are entitled to have your taste, I have to point out that Jeff
is just as entitled to their taste, and I don't think that you can claim
that yours is better.
So I wonder what the intended outcome of this review is? To make the patch
better? Or to pit taste against taste?
Ciao,
Johannes
