On Thu, Sep 30, 2021 at 02:55:16PM +0200, Kevin Wolf wrote: > > > When we're changing these lines anyway, let's make sure to have > > > consistent alignment with the surrounding code. So I would prefer > > > something like: > > > > > > + .bdrv_close = parallels_close, > > > .bdrv_child_perm = bdrv_default_perms, > > > > > > Rather than: > > > > > > + .bdrv_close = parallels_close, > > > .bdrv_child_perm = bdrv_default_perms, > > > > > > In most cases, there are already inconsistencies in the BlockDriver > > > definitions, but let's use the chance to make it a little better. > > > > > > Or may be drop alignment around '=' at all, to have > > > > .bdrv_child_perm = bdrv_default_perms, > > .bdrv_co_block_status = parallels_co_block_status, > > .bdrv_has_zero_init = bdrv_has_zero_init_1, > > > > ? > > You're right that this would make it easy to keep things consistent, but > I think it hurts readability a lot, even compared to the current, often > inconsistent state.
I agree that the alignment adds a bit of readability, but also understand that it adds a maintenacne burden. Thus, in code I manage, I'm fine with either style (no extra spaces, or '=' lined up); and can live with different styles in different files (which I then will honor when doing a grunt-work patch that touches all of the block drivers). But what I don't like is when a single file cannot be consistent with itself on which of those two styles it is using - a file that uses aligned '=' really needs to put that '=' far enough to the right that an added long-named member doesn't cause frequent reindentation of the rest of the members. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
