Re: [PATCH v2 0/7] Builtin FSMonitor Part 1

On Thu, Sep 23, 2021 at 04:33:20PM +0200, Ævar Arnfjörð Bjarmason wrote:

E.g. here (just an example that includes Taylor, since he reviewed v1
here) is a case where Taylor suggested something that I didn't go for,
but i'd like to think noting it helped him catch up:
https://lore.kernel.org/git/cover-v4-0.5-00000000000-20210921T131003Z-avarab@gmail.com/ (local)

It did, but to be honest I would have been totally fine with you
mentioning the changes you did incorporate into the rerolled version,
and omitting mention of any insignificant suggestions you decided to
ignore.

In other words, when I got to the same spot in the rerolled version, I
would have either thought "looks like Ævar didn't take my suggestion,
OK" or not have remembered it in the first place. Either way, the point
was trivial enough that I didn't bother to pursue it further in that
thread.

And I think that's what is happening here, too. Yes, I find the forward
declaration useless on GCC, though it appears to be helpful on MSVC and
hurtful on clang. Even if it does produce a strictly worse error message
on clang, do we really care? It may cause some mild inconvenience for a
developer later on, but I find it highly unlikely that it would allow us
to ship a bug that wouldn't have been caught one way or another during
development.

So I was a little disappointed to see such a back-and-forth about this
quite trivial point. I realize that I'm piling on here by adding my
two-cents, but I think it's worth it to ask ourselves more often what
points we're willing to concede and which are worth advocating more
strongly for.

Thanks,
Taylor