[Top] [All Lists]

Re: [PATCH 3/3 v2] xfstests: Add support for sections in config file

To: LukÃÅ Czerner <lczerner@xxxxxxxxxx>
Subject: Re: [PATCH 3/3 v2] xfstests: Add support for sections in config file
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Mon, 1 Jul 2013 19:12:56 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <alpine.LFD.2.00.1307011022300.2542@xxxxxxxxxxxxxxxxxxxxx>
References: <1372426320-19902-1-git-send-email-lczerner@xxxxxxxxxx> <1372426320-19902-3-git-send-email-lczerner@xxxxxxxxxx> <20130701014944.GD27780@dastard> <alpine.LFD.2.00.1307011022300.2542@xxxxxxxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Mon, Jul 01, 2013 at 10:36:37AM +0200, LukÃÅ Czerner wrote:
> On Mon, 1 Jul 2013, Dave Chinner wrote:
> > > Signed-off-by: Lukas Czerner <lczerner@xxxxxxxxxx>
> > > ---
> > >  check         | 371 
> > > +++++++++++++++++++++++++++++++++-------------------------
> > >  common/config | 126 ++++++++++++--------
> > >  common/rc     |  63 +++++-----
> > >  3 files changed, 326 insertions(+), 234 deletions(-)
> > 
> > This patch probably needs to be broken up, too. A substantial part
> > of it is indentation changes, which probably should be split into 2
> > parts - factor code into function, then wrap loop around function.
> > The change to the summary information should be done as a separate
> > patch, too. I suspect many of the common/config changes coul dbe
> > split up, too.
> Ok, I'll see how can I split it up. Btw, indentation in xfstests is
> a mess, because sometimes we're using spaces and sometimes tabs. Is
> there any preference ? (I would definitely prefer tabs)


> > The changing of the $RESULT_BASE should probably also
> > bein a separate patch, because this is something that we'll need to
> > discuss as it changes the structure of the output....
> I am not sure it should be separate from this patch because the new
> structure will only be used if the new config format (with sections)
> is used.

Which is a bit confusing, especially as a separate result directory
might be desired for the output of each section - say I want to keep
all the "config A" results together, but spearate to all the "config
B" results. Placing them all under the same $RESULT_BASE isn't ideal
at that point....

IOWs, I suspect that $RESULT_BASE should be able to be defined in
the section config so that you can redirect results that way.

> > Oh, and why make a distinction between no sections and
> > $OPTIONS_HAVE_SECTIONS in the config file? Surely no sections is
> > just the same as having 1 section....
> Yes, but it's much "nicer" to check boolean option than checking
> what is the name of the first section. Really this is just a
> workaround, because I did not want to change result structure and
> output if one is not using the new config format.

That's another reason why I think that we should be able to have a
per-section RESULTS_BASE - that way the code doesn't care how
RESULTS_BASE is defined or where it points to - the output of
xfstests is *always* the same. That is, after all, why $RESULTS_BASE
was introduced in the first place.

> It could be done
> so that when there are no sections we'll always use "default" section,
> not sure what would people prefer.

Exactly why it needs discussion ;)


Dave Chinner

<Prev in Thread] Current Thread [Next in Thread>