From: Dave Chinner <david@fromorbit.com>
To: Phil White <pwhite@sgi.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 02/25] xfstests: remove bench infrastructure
Date: Wed, 20 Mar 2013 09:31:49 +1100 [thread overview]
Message-ID: <20130319223149.GA17758@dastard> (raw)
In-Reply-To: <20130315193635.GA2025@caliban.engr.sgi.com>
On Fri, Mar 15, 2013 at 12:36:38PM -0700, Phil White wrote:
> On Fri, Mar 15, 2013 at 11:27:46PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > The benchmark framework inside xfstests is basically unused,
> > bitrotted and not very useful. If we need benchmarks, lets use a
> > real benchmark framework, not xfstests. Kill it to remove
> > dependencies on common and common.rc.
> >
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
>
> I've been straightforward and clear in why I reposted the patch set and why
> I'm personally interested in the benchmark framework staying around. I don't
> see value in explaining it again.
I understand why you see value in a benchmark infrastructure, but
that's not the issue here. The issue at hand is whether we should be
trying to maintain arbitrary abstractions that are made redundant by
the cleanup patch in the hope they are useful in the future.
There are solid technical reasons for what I've done, but you
haven't provided any to advocate why we should accept your version
as a better solution. "personal interest" is a good thing to have,
but it's not a convincing technical argument....
FWIW, there's an example of this remove/re-implement process I
advocate for cleanups in this patchset. I removed the expunged file
support because it makes infrastructure modifications simpler. I
then re-introduce an enhanced version of the same functionality
later in the series after all the structural changes have been made.
The result is a cleaner, more functional implementation. That would
not have occurred had I simply tried to keep the existing support
and then hack new functionality into it. Instead, the separation of
old code removal and re-implementation in the new infrastructure has
lead to a better implementation and cleaner code.
There are no obstacles to making new abstractions as needed by the
new bench infrastructure and it's trivial to do as required by new
code. The result will be cleaner, more maintainable code, and that's
the compelling reason for removing the old abstractions before
introducing new dependencies.
Hence what I'm asking you is this: why you can't follow this same
process for your new benchmarking infrastructure? i.e. What makes the
existing abstaction so compelling that we need to keep it?
> I don't see how it serves anyone. I don't see how it serves the community
> to repost this rather than excluding this change, pushing this through, and
> improving xfstests. The resultant exchange of mail does nothing but to
> litter everyone else's inbox.
Ignoring the abstraction differences, the version I posted has new
functionality, is current with all the tests in the repository
(except now for the 313->306 renaming) and has several bug fixes
compared to the version you rebased.
If you consider that litter, then, well, .... <sigh>
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2013-03-19 22:32 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-15 12:27 [PATCH 00/25] xfstests: xfstests: move tests out of top level Dave Chinner
2013-03-15 12:27 ` [PATCH 01/25] xfstests: remove remake script Dave Chinner
2013-03-22 16:51 ` Phil White
2013-03-15 12:27 ` [PATCH 02/25] xfstests: remove bench infrastructure Dave Chinner
2013-03-15 19:36 ` Phil White
2013-03-19 22:31 ` Dave Chinner [this message]
2013-03-21 23:52 ` Phil White
2013-03-22 7:37 ` Dave Chinner
2013-03-22 16:22 ` Troy McCorkell
2013-03-26 20:26 ` Troy McCorkell
2013-03-22 16:53 ` Phil White
2013-03-15 12:27 ` [PATCH 03/25] xfstests: kill useless test owner fields Dave Chinner
2013-03-22 16:57 ` Phil White
2013-03-15 12:27 ` [PATCH 04/25] xfstests: remove stale machine configs Dave Chinner
2013-03-22 17:02 ` Phil White
2013-03-15 12:27 ` [PATCH 05/25] xfstests: fold common into check Dave Chinner
2013-03-23 10:18 ` Phil White
2013-03-15 12:27 ` [PATCH 06/25] xfstests: clean up and simply check CLI option parsing Dave Chinner
2013-03-23 10:19 ` Phil White
2013-03-15 12:27 ` [PATCH 07/25] xfstests: kill hangcheck stuff from check Dave Chinner
2013-03-23 10:19 ` Phil White
2013-03-15 12:27 ` [PATCH 08/25] xfstests: remove test expunge file support Dave Chinner
2013-03-23 10:19 ` Phil White
2013-03-15 12:27 ` [PATCH 09/25] xfstests: remove undocumented TESTS_REMAINING_LOG Dave Chinner
2013-03-23 10:20 ` Phil White
2013-03-15 12:27 ` [PATCH 10/25] xfstests: include test subdirectory support Dave Chinner
2013-03-23 10:20 ` Phil White
2013-03-15 12:27 ` [PATCH 11/25] xfstests: remove 285.full and 286.full Dave Chinner
2013-03-23 10:20 ` Phil White
2013-03-15 12:27 ` [PATCH 12/25] xfstests: move generic tests out of top level dir Dave Chinner
2013-03-23 10:22 ` Phil White
2013-03-15 12:27 ` [PATCH 13/25] xfstests: move xfs specific tests out of top directory Dave Chinner
2013-03-23 10:22 ` Phil White
2013-03-15 12:27 ` [PATCH 14/25] xfstests: move remaining tests out of top level directory Dave Chinner
2013-03-23 10:23 ` Phil White
2013-03-15 12:27 ` [PATCH 15/25] xfstests: rework CLI individual test specification Dave Chinner
2013-03-23 10:23 ` Phil White
2013-03-15 12:28 ` [PATCH 16/25] xfstests: make exclude groups aware of multiple subdirectories Dave Chinner
2013-03-23 10:23 ` Phil White
2013-03-15 12:28 ` [PATCH 17/25] xfstests: Introduce a results directory Dave Chinner
2013-03-23 10:23 ` Phil White
2013-03-15 12:28 ` [PATCH 18/25] xfstests: convert tests to use new " Dave Chinner
2013-03-23 10:24 ` Phil White
2013-03-15 12:28 ` [PATCH 19/25] xfstests: fix _link_out_file callers Dave Chinner
2013-03-23 10:24 ` Phil White
2013-03-15 12:28 ` [PATCH 20/25] xfstests: introduce a common directory Dave Chinner
2013-03-23 10:24 ` Phil White
2013-03-15 12:28 ` [PATCH 21/25] xfstests: Reintroduce configurable test expunging Dave Chinner
2013-03-23 10:24 ` Phil White
2013-03-15 12:28 ` [PATCH 22/25] xfstests: RESULTS_DIR needs to be an absolute path Dave Chinner
2013-03-23 10:24 ` Phil White
2013-03-15 12:28 ` [PATCH 23/25] xfstests: Decomplicate quota setup in 050 Dave Chinner
2013-03-23 10:24 ` Phil White
2013-03-15 12:28 ` [PATCH 24/25] xfstests: clean up test 262 output file use Dave Chinner
2013-03-23 10:24 ` Phil White
2013-03-15 12:28 ` [PATCH 25/25] xfstests: use _notrun for tape checks Dave Chinner
2013-03-23 10:24 ` Phil White
2013-03-27 10:51 ` [PATCH 00/25] xfstests: xfstests: move tests out of top level Rich Johnston
2013-03-27 10:51 ` Rich Johnston
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=20130319223149.GA17758@dastard \
--to=david@fromorbit.com \
--cc=pwhite@sgi.com \
--cc=xfs@oss.sgi.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