linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Pavel Reichl <preichl@redhat.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH v2 00/29] xfsprogs: Drop the 'platform_' prefix
Date: Sat, 7 Aug 2021 09:05:01 +1000	[thread overview]
Message-ID: <20210806230501.GG2757197@dread.disaster.area> (raw)
In-Reply-To: <20210806212318.440144-1-preichl@redhat.com>

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

  parent reply	other threads:[~2021-08-06 23:05 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-06 21:22 [PATCH v2 00/29] xfsprogs: Drop the 'platform_' prefix Pavel Reichl
2021-08-06 21:22 ` [PATCH v2 01/29] xfsprogs: Stop using platform_uuid_compare() Pavel Reichl
2021-08-06 21:22 ` [PATCH v2 02/29] xfsprogs: Stop using platform_test_xfs_fd() Pavel Reichl
2021-08-06 21:22 ` [PATCH v2 03/29] xfsprogs: Stop using platform_test_xfs_path() Pavel Reichl
2021-08-06 21:22 ` [PATCH v2 04/29] xfsprogs: Stop using platform_fstatfs() Pavel Reichl
2021-08-06 21:22 ` [PATCH v2 05/29] xfsprogs: Stop using platform_getoptreset() Pavel Reichl
2021-08-06 21:22 ` [PATCH v2 06/29] xfsprogs: Stop using platform_uuid_copy() Pavel Reichl
2021-08-06 21:22 ` [PATCH v2 07/29] xfsprogs: Stop using platform_uuid_generate() Pavel Reichl
2021-08-06 21:22 ` [PATCH v2 08/29] xfsprogs: Stop using platform_uuid_unparse() Pavel Reichl
2021-08-06 21:22 ` [PATCH v2 09/29] xfsprogs: Stop using platform_uuid_clear() Pavel Reichl
2021-08-06 21:22 ` [PATCH v2 10/29] xfsprogs: Stop using platform_uuid_parse() Pavel Reichl
2021-08-06 21:23 ` [PATCH v2 11/29] xfsprogs: Stop using platform_uuid_is_null() Pavel Reichl
2021-08-06 21:23 ` [PATCH v2 12/29] xfsprogs: Stop using platform_check_mount() Pavel Reichl
2021-08-06 21:23 ` [PATCH v2 13/29] xfsprogs: Stop using platform_check_ismounted() Pavel Reichl
2021-08-06 21:23 ` [PATCH v2 14/29] xfsprogs: Stop using platform_flush_device() Pavel Reichl
2021-08-06 21:23 ` [PATCH v2 15/29] xfsprogs: Stop using platform_mntent_open() Pavel Reichl
2021-08-06 21:23 ` [PATCH v2 16/29] xfsprogs: Stop using platform_mntent_next() Pavel Reichl
2021-08-06 21:23 ` [PATCH v2 17/29] xfsprogs: Stop using platform_mntent_close() Pavel Reichl
2021-08-06 21:23 ` [PATCH v2 18/29] xfsprogs: Stop using platform_findsizes() Pavel Reichl
2021-08-06 21:23 ` [PATCH v2 19/29] xfsprogs: Stop using platform_discard_blocks Pavel Reichl
2021-08-06 21:23 ` [PATCH v2 20/29] xfsprogs: Stop using platform_zero_range() Pavel Reichl
2021-08-06 21:23 ` [PATCH v2 21/29] xfsprogs: Stop using platform_crash() Pavel Reichl
2021-08-06 21:23 ` [PATCH v2 22/29] xfsprogs: Stop using platform_nproc() Pavel Reichl
2021-08-06 21:23 ` [PATCH v2 23/29] xfsprogs: Stop using platform_check_iswritable() Pavel Reichl
2021-08-06 21:23 ` [PATCH v2 24/29] xfsprogs: Stop using platform_set_blocksize() Pavel Reichl
2021-08-06 21:23 ` [PATCH v2 25/29] xfsprogs: Stop using platform_findrawpath() Pavel Reichl
2021-08-06 21:23 ` [PATCH v2 26/29] xfsprogs: Stop using platform_findblockpath() Pavel Reichl
2021-08-06 21:23 ` [PATCH v2 27/29] xfsprogs: Stop using platform_direct_blockdev() Pavel Reichl
2021-08-06 21:23 ` [PATCH v2 28/29] xfsprogs: Stop using platform_align_blockdev() Pavel Reichl
2021-08-06 21:23 ` [PATCH v2 29/29] xfsprogs: Stop using platform_physmem() Pavel Reichl
2021-08-06 23:05 ` Dave Chinner [this message]
2021-08-07 15:13   ` [PATCH v2 00/29] xfsprogs: Drop the 'platform_' prefix Pavel Reichl
2021-08-07 21:46     ` Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210806230501.GG2757197@dread.disaster.area \
    --to=david@fromorbit.com \
    --cc=linux-xfs@vger.kernel.org \
    --cc=preichl@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).