Peter Maydell <peter.mayd...@linaro.org> writes: > On Tue, 22 Nov 2022 at 08:58, Markus Armbruster <arm...@redhat.com> wrote: >> I don't think complete detailed review is necessary or even sensible. >> >> Review should start with the Coccinelle script: >> >> // replace 'R = X; return R;' with 'return X;' >> @@ >> identifier VAR; >> expression E; >> type T; >> identifier F; >> @@ >> T F(...) >> { >> ... >> - T VAR; >> ... when != VAR >> >> - VAR = (E); >> - return VAR; >> + return E; >> ... when != VAR >> } >> >> What could go wrong? Not a rhetorical question! > > The obvious answer is "you might have got your manual tweaking > wrong". A purely mechanised patch I can review by looking at > the script and maybe eyeballing a few instances of the change; > a change that is 99% mechanised and 1% hand-written I need to > run through to find the hand-written parts.
Define "handwritten" :) If reverting unwanted line-breaks and blank lines counts, then I can make two patches, one straight from Coccinelle, and one that reverts the unwanted crap. The first one will be larger and more annoying to review than this one. A clear loss in my book, but I'm the patch submitter, not a patch reviewer, so my book doesn't matter. Else, we're down to one file, which I already offered to split off. > But mostly this patch is hard to review for its sheer size, > mechanical changes or not. A 3000 line patchmail is so big that > the UI on my mail client gets pretty unwieldy. With the manual one split off, target/xtensa/ dropped as requested by Max, and tests/tcg/mips/ dropped because its status is unclear (and I start to find it hard to care), we're down to 28 files changed, 81 insertions(+), 221 deletions(-) This will be v2.