Thread (581 messages) flat view 581 messages, 16 authors, 2021-10-12

Re: [PATCH 4/9] hook: treat hookdir hook specially

From: Eric Sunshine <hidden>
Date: 2021-07-23 18:22:52

On Fri, Jul 23, 2021 at 1:34 PM Felipe Contreras
[off-list ref] wrote:
Emily Shaffer wrote:
quoted
On Fri, Jul 16, 2021 at 10:58:34AM +0200, Ævar Arnfjörð Bjarmason wrote:
quoted
At this point I tihnk it would be way better to squash this and other
such changes that basically add a field to a struct that isn't used yet
into whatever commit use/need them.
I think at this point we run into you and me having different
patch-storytelling styles - which probably is what led to the big topic
restart in the first place ;)
Yes, but as a reader of the story I prefer not to have to read the
entire thing in order to understand it. I prefer each page to tell a
small story.

Putting my armchair reviewer hat I cannot do that for this particular
patch, I would need to do more work to make sense of it, and while I'm
writing this message to explains that, others will simply skip it, and
that's a lost opportunity.
Implicit in what Felipe and Ævar are saying is that a well-structured
patch series asks the reviewer to keep only one or two details in mind
after reading a patch in order to understand the next patch in the
series, and that the reviewer shouldn't be expected to keep a large
set of details in mind over several patches. Unlike the author of the
patches who can keep all the details in mind at once and understands
the series in its entirety, reviewers (usually) don't have such
luxury[1]. So, it's important to hand-hold reviewers as much as
possible by not asking them to remember a lot of details between
patches and by ensuring that the details which they must remember only
need to be remembered for a very short time. This is why it is
helpful, for instance, to bundle documentation and test updates in the
patch with changes to code, so the reviewer can see at a glance that
the changes to documentation and tests match the changes to the code,
rather than delaying documentation and test updates until later in the
series.

[1]: If you've ever read a novel in which the author has multiple
story lines running and switches between story lines infrequently,
such that when the author switches back to story-line "A", which you
last saw 100 pages ago, and you can't remember what was going in "A"
or even who the minor characters are anymore, so that you have to go
back and reread 10 or 20 pages from the previous time you saw "A",
then that's representative of the difficulty reviewers can experience
when reading a patch series, except with a patch series, the cognitive
load is already quite high. (Very nice run-on sentence I just wrote.)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help