Date: Tue, 9 Feb 2021 16:35:59 -0700 From: Warner Losh <imp@bsdimp.com> To: John Baldwin <jhb@freebsd.org> Cc: Alexey Dokuchaev <danfe@freebsd.org>, Mateusz Guzik <mjguzik@gmail.com>, Jessica Clarke <jrtc27@freebsd.org>, Kevin Bowling <kevin.bowling@kev009.com>, "src-committers@freebsd.org" <src-committers@freebsd.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@freebsd.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@freebsd.org> Subject: Re: git: af366d353b84 - main - amd64: implement strlen in assembly Message-ID: <CANCZdfrkVbkj%2BW5zkgtOovMFsseuvUCbCtXbWfE-2ZhfoN%2BMXQ@mail.gmail.com> In-Reply-To: <3e6dfdae-48a8-2d6c-1f42-c92554d74f82@FreeBSD.org> References: <202102081915.118JFXkJ067892@gitrepo.freebsd.org> <3E64387A-42DD-4470-8893-5B774F19754E@freebsd.org> <CAK7dMtA5rbT6QoCK2HQfzLY2f%2B3kbhdJp0ERmRpc_CVAog%2B6bg@mail.gmail.com> <8E61EA5C-39D1-49CC-8319-06E9192FF735@freebsd.org> <CAGudoHG%2BiKB4q=QoFEadtHsXy9ODX%2BkEHn-hU71F8yGEbzP5MA@mail.gmail.com> <20210209145348.GA70871@FreeBSD.org> <3e6dfdae-48a8-2d6c-1f42-c92554d74f82@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Feb 9, 2021 at 3:17 PM John Baldwin <jhb@freebsd.org> wrote: > 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. > Just to draw back to the main point I was trying to make: Let's not let the perfect be the enemy of the better. Reviews are better today than we were last year and even better than two years ago. While we still have a ways to go, we've gotten better and will continue to get better as more people give things another try. Warner
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfrkVbkj%2BW5zkgtOovMFsseuvUCbCtXbWfE-2ZhfoN%2BMXQ>