Re: [PATCH 00/42] mkfs: factor the crap out of the code

[Dave Chinner <david@fromorbit.com>]

On Thu, Aug 31, 2017 at 12:10:56AM +0200, Luis R. Rodriguez wrote:
> On Wed, Aug 30, 2017 at 03:44:20PM +1000, Dave Chinner wrote:
> > On Wed, Aug 30, 2017 at 06:16:35AM +0200, Luis R. Rodriguez wrote:
> > > On Wed, Aug 30, 2017 at 09:50:10AM +1000, Dave Chinner wrote:
> > > > Everyone who tries to modify mkfs quickly learns that it is a pile
> > > > of spaghetti, the only difference in opinion is whether it is a
> > > > steaming, cold or rotten pile. This patchset attempts to untangle
> > > > the ball of pasta and turn it into a set of clear, obvious
> > > > operations that lead to a filesystem being formatted correctly.
> > > 
> > > Yay.
> > > 
> > > > The patch series is really in three parts, splitting the code up
> > > > into roughly three modules.
> > > 
> > > Any reason you ended up with 3 instead of 4 as originally envisioned?
> > 
> > Because the 4th module - config file support - doesn't exist yet.
> 
> To be fair you had itemized before:
> 
>   1) Settings default - struct mkfs_default_params

As I said: this module doesn't exist yet, so it's output struct
mkfs_default_params is statically defined (i.e. built in defaults)
to provide input to the next module. Hence there are only three
modules in this patchset:

>   2) CLI parsing - struct cli_params
>   3) Validation + calculation - struct mkfs_params
>   4) On disk formatting

[....]

> > Maybe you can come up with a way of automating this, but for a
> > one-off piece of work that affects a point-in-time snapshot of mkfs
> > functionality, I'm not sure it's worth the effort to try to make a
> > generic test to do this sort of thing.
> 
> In Dave we trust!

No, definitely don't do that.  Don't trust a damn thing I do - I'm
full of dangerous ideas, I write shit code and should be kept on a
short leash at all times.....

> > > > finally, one for config file support),
> > > > but otherwise the majority of the factoring work is now complete.
> > > > 
> > > > Comments, flames, etc all welcome.
> > > 
> > > Just one thing, got a git tree I can use? I honestly can't be bothered
> > > reviewing the delta in between, I just want to move on with life. Thanks
> > > for cleaning up the manure pile buttress.
> > 
> > Nope, not right now. Tag all the patches, save them to an mbox
> > file, run 'git-am <mbox-file>' to apply them all. Takes all of 20s
> > to do with mutt....
> 
> Turns out mutt re-orders tagged messages

It does? I've never come across that. I always tag the entire thread
they've always come out in the correct sent order for me. And I've
been doing that for many, many years....

> in what I think may be the order you got them in so the order on
> the input output filename may differ from the patch order intent.
> Even when I manually sort them and apply them, the patches failed
> on both origin/master and origin/for-next, so I must be doing
> something wrong or using an incorrect branch or commit ID. What
> branch and commit ID should I use?

Applies to:

$ glo -n 1 origin/for-next
3540b418ba48 xfs_db: btdump should avoid eval for push and pop of cursor
$

I forgot to update the branch before my last pass over the
patchset...

> It also seems I didn't get patch #20 in my inbox, could you
> resend?

Probably best in one-off cases like this is to grab it from the
archive - the whole patchset made it back to me from the list, so it
should be in the archive. If it's not, then we've got a more general
list delivery problem rather than it just being a problem somewhere
in your mail delivery path...


Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com