Re: [kvm-unit-tests PATCH v3 01/27] lib: Fix style for acpi.{c,h}

[Andrew Jones <andrew.jones@linux.dev>]

(Dropping the drjones@redhat.com address from CC since, as of today,
it's generating auto address-not-valid messages.)

On Fri, Jul 01, 2022 at 10:52:28AM +0100, Nikos Nikoleris wrote:
> Hi Drew
> 
> Thanks for the review!
> 
> On 01/07/2022 10:27, Andrew Jones wrote:
> > Hi Nikos,
> > 
> > I guess you used Linux's scripts/Lindent or something for this
> > conversion. Can you please specify what you used/did in the
> > commit message?
> > 
> 
> I fixed the style by hand but happy to use Lindent in the next iteration.

Yeah, I recommend it. I used it for commit 0e9812980ee5 ("lib: Fix
whitespace"). I did need to modify it to allow 100 columns instead of 80,
though. Also, I then looked at the changes with 'git diff -b' and saw a
few other things to modify by hand.

> 
> > On Thu, Jun 30, 2022 at 11:02:58AM +0100, Nikos Nikoleris wrote:
> > > Signed-off-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
> > > ---
> > >   lib/acpi.h | 148 ++++++++++++++++++++++++++---------------------------
> > >   lib/acpi.c |  70 ++++++++++++-------------
> > >   2 files changed, 108 insertions(+), 110 deletions(-)
> > 
> > It looks like the series is missing the file move patch. Latest master
> > still doesn't have lib/acpi.*
> > 
> 
> I am sorry, I missed the first patch. The missing patch is doing a move of
> acpi.{h,c} [1].
> 
> FWIW, I tried combining the patches in one but I ended up with a big diff. I
> found it much easier to check that everything looks ok when the overall
> change was split in two patches.

Yes, please do it in two separate patches with the move patch generated
with -M, as you've done in [1].

Thanks,
drew

> 
> [1]: https://github.com/relokin/kvm-unit-tests/commit/959ca08c23dbaa490b936303b94b006352a29d43
> 
> Thanks,
> 
> Nikos
> 
> > Thanks,
> > drew