Re: [PATCH v2 00/29] xfsprogs: Drop the 'platform_' prefix

Dave Chinner
2021-08-06T23:05:06Z

On Fri, Aug 06, 2021 at 11:22:49PM +0200, Pavel Reichl wrote:
> Stop using commands with 'platform_' prefix. Either use directly linux
> standard command or drop the prefix from the function name.

Looks like all of the patches in this series are missing
signed-off-by lines. Most of them have empty commit messages, too,
which we don't tend to do very often.

> 51 files changed, 284 insertions(+), 151 deletions(-)

IMO, 29 patches for such a small change is way too fine grained for
working through efficiently. Empty commit messages tend to be a
sign that you can aggregate patches together.... i.e. One patch for
all the uuid changes (currently 7 patches) with a description of why
you're changing the platform_uuid interface, one for all the mount
related stuff (at least 5 patches now), one for all the block device
stuff (8 or so patches), one for all the path bits, and then one for
whatever is left over.

Every patch has overhead, be it to produce, maintain, review, test,
merge, etc. Breaking stuff down unnecessarily just increases the
amount of work everyone has to do at every step. So if you find that
you are writing dozens of patches that each have a trivial change in
them that you are boiler-plating commit messages, you've probably
made the overall changeset too fine grained.

Also....

> libxfs/init.c | 32 ++++++------
> libxfs/libxfs_io.h | 2 +-
> libxfs/libxfs_priv.h | 3 +-
> libxfs/rdwr.c | 4 +-
> libxfs/xfs_ag.c | 6 +--
> libxfs/xfs_attr_leaf.c | 2 +-
> libxfs/xfs_attr_remote.c | 2 +-
> libxfs/xfs_btree.c | 4 +-
> libxfs/xfs_da_btree.c | 2 +-
> libxfs/xfs_dir2_block.c | 2 +-
> libxfs/xfs_dir2_data.c | 2 +-
> libxfs/xfs_dir2_leaf.c | 2 +-
> libxfs/xfs_dir2_node.c | 2 +-
> libxfs/xfs_dquot_buf.c | 2 +-
> libxfs/xfs_ialloc.c | 4 +-
> libxfs/xfs_inode_buf.c | 2 +-
> libxfs/xfs_sb.c | 6 +--
> libxfs/xfs_symlink_remote.c | 2 +-

Why are all these libxfs files being changed?

$ git grep -l platform libxfs/
libxfs/init.c
libxfs/libxfs_io.h
libxfs/libxfs_priv.h
libxfs/rdwr.c
libxfs/xfs_log_format.h
$

IOWs, there are calls to platform_*() functions in most of these
libxfs files, so what is being changed here? If these are code
changes, then they will end up needed to be ported across to the
kernel libxfs, too.

I did a quick scan of a few of the patches but I didn't land on the
one that had these changes in it; another reason for not doing stuff
in such fine grained ways. Hence it's not clear to me why renaming
platform_*() functions would touch these files.

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com