Re: [PATCH v3 02/34] fsmonitor--daemon: man page

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 2570 bytes --]

Hi Ævar,

On Tue, 13 Jul 2021, Ævar Arnfjörð Bjarmason wrote:

> [snip Ævar's suggestion to populate the manual page incrementally,
> interspersed with the commits that finalize implementing the respective
> functionality]
>
> I suggested this because I think it's much easier to read patches that
> are larger because they incrementally update docs or tests along with
> code, than smaller ones that are e.g. "add docs" followed by
> incrementally modifying the code.

My experience is the exact opposite of yours: shorter patches are easier
to read.

> That's because you can consider those atomically.

No, in a patch series you cannot consider any patch completely atomically.
Just like you don't consider any paragraph in any well-written book out of
context.

> If earlier doc changes refer to later code changes you're left jumping
> back & forth and wondering if the code you're reading that doesn't match
> the docs yet is a bug, or if it's solved in some later change you're now
> needing to mentally keep track of.

You only keep jumping back and forth when reviewing _patches_. We try to
do code review on this mailing list, which means that you have the code
locally and review it in the correct context. When you do that, a design
document is quite helpful. And the proposed manual page serves as such a
design document.

> Which is not some theoretical concern b.t.w., but exactly what I found
> myself doing when reading this series, hence the suggestion.

Part of the problem here seems to be that this patch series saw many
reviewer suggestions that forced it to increase in length. That was
probably not very helpful, after all.

The first iteration had 23 patches. Reviews forced it to grow to 28
patches in the second iteration. And that was still not enough, therefore
the third iteration consisted of 34 patches. And if you had had your way,
requiring Jeff to include a Linux backend in the same patch series, it
would have to increase in size again, and not just by a little.

This all sounds like we're truly falling into the trap of ignoring the
rule that the perfect is the enemy of the good.

I really would like to come back to a focused review that truly improves
the patches at hand, and avoids conflating the review of the actual patch
series with matters of personal taste (which is a recipe for
disagreement). After all, we are interested in getting this feature out to
users who need to work with very large worktrees, right? At least that's
my goal here.

Ciao,
Johannes