git: af366d353b84 - main - amd64: implement strlen in assembly

[John Baldwin]

On 2/9/21 6:53 AM, Alexey Dokuchaev wrote:
> On Tue, Feb 09, 2021 at 02:41:15PM +0100, Mateusz Guzik wrote:
>> ...
>> More, if reviews were mandatory, I would expect their quality to go
>> down even further, making them even less likely to prevent breakage.
> 
> Exactly that.  In fact, the good reviews are typically coming from
> people who care.  But those you'll get regardless of whether you've
> asked for them.

No, that's not quite true.  Committing without asking for any review at
all is not the same as requesting review and then timing out when it
doesn't occur.

Also, as has been noted multiple times now, people do point out questions
that can't easily be fixed post-commit such as too-terse commit logs.
Those are quite easily caught in review if one makes the effort to ask.

If we want to cherry-pick examples, we can also find examples where
reviews do find issues pre-commit.  Look at all the back and forth on
Warner's doc change about libraries and symbol versioning in D28486 for
an example.

The discussion in D28453 has led to a better approach I still need to
update the review with that moves the handling of pollable sims one
layer up.

Rob Wing found an issue I had missed in my bhyve config change (D26035)
in terms of new warnings from iasl during pre-commit testing.

Kostik posted a possible patch trying to address a PR in D28485 that is
probably not valid (see my review comments) and thus saved having
something committed that then had to be reverted.

Kostik's review on D28342 forced me to rework the change to only scan
segments and not ELF sections since valid ELF executables and DSO's
aren't required to have section headers.

Review on D27454 led to acclerated AES-GCM for ARMv8 that was targeted
at KTLS being reimplemented in a more generic fashion that also
accelerates IPsec and other users of AES-GCM in the kernel.

I could keep going listing changes that benefit from cooperation among
developers.  However, cooperation does mean one has to be a bit more
patient and be willing to work on follow-on work while letting review
feedback come in.  Using tools like git make this fairly easy as you
can apply fixups to the earlier changes and rebase the follow-on changes
under active development afterwards.  It is true that you can't get
meaningful review on all changes (or all aspects of a change), but I
think the notion that review _never_ helps is not supported by the
evidence.

The fact that Jess found a bug in the assembly code in question the day
after it was committed indicates that pre-commit review would have
been beneficial for this commit.

-- 
John Baldwin