Re: [Libguestfs] [libnbd PATCH 00/18] wrap hand-written source code at 80 characters
On Tue, Apr 18, 2023 at 07:26:13PM +0200, Laszlo Ersek wrote:
[I started this email yesterday, then postponed it while looking at
individual patches...]
This series wraps the non-generated C-language source code (*.c and *.h
files) at 80 characters.
"ocaml/helpers.c" remains overlong, but I couldn't find a way to wrap
it: its single overlong line contains the comment
/* For how we're getting the exception name, see:
*
https://github.com/libguestfs/libguestfs/blob/5d94be2583d557cfc7f8a8cfee7...
*/
and even if I truncate the blob hash to 12 nibbles, the line remains too
long.
Truncating is fine to reduce the worst of the width, but I also
understand your reluctance to trim too short (git defaults to 7
nibbles in a fresh repository, but larger repositories like linux.git
output at least 10 nibbles and sometimes more because there are just
that many more hash prefix collisions as history grows - it's never
fun when a link valid today stops working tomorrow when a prefix
collision is introduced into the repo). At any rate, I have no
problems with long URLs in source files that are otherwise
length-constrained.
The following files are also too wide:
include/libnbd.h
lib/api.c
lib/states-run.c
lib/states.c
lib/states.h
lib/unlocked.h
ocaml/nbd-c.c
python/libnbdmod.c
python/methods.h
but they are all generated; we'll have to discuss them separately.
Wrapping a generated file for legibility is definitely harder work;
legible generated code still has its benefits, but longer generated
lines for faster coding of the generator is a tolerable tradeoff in my
book.
Overall, the series looked okay to me at a first read through; I did
spot some things on individual patches where I made comments, but they
are of the nature where I'm also okay with you adding:
Reviewed-by: Eric Blake <eblake(a)redhat.com>
whether or not you touch things up.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org