[parted-devel] [PATCH] mkpartfs ext2 2 10 would erroneously report "file system too small"

[David Cantrell]

On Fri, 2007-05-18 at 15:18 -0300, Otavio Salvador wrote:
> David Cantrell <dcantrell at redhat.com> writes:
> 
> > Otavio, Jim.... First, you both have good points here, but I think we've
> > missed something here with our unit testing framework.  Ideally, the
> > developer shouldn't also be the one to write all the testing.  For
> > maximum testing effectiveness, unit tests should be written by another
> > person rather than the author.  We should think about this going
> > forward.
> 
> Right I agree that it's the ideal but it's hard to get there for a FS
> project.

Hard, but not impossible.

> > We also need to take in to account that the parted code base is not in
> > great shape.  It's had many hands in it and we all are trying to bring
> > it up to date and correct a lot of defects.  Jim's patch clears a make
> > distcheck failure which, in my opinion, is more important in the short
> > term than a unit test for the function.
> 
> I agree completely with you while I think we should at least wait for
> a consensus _before_ commit.

Sure.  In this case, I think Jim was assuming this was a relatively
trivial fix and everyone was on the same page for it.

> > As development progresses and we begin making more substantial changes
> > to parted, we can start enforcing more strict policies such as requiring
> > full unit testing for a new function at commit time.  For now, I think
> > we should all be a little more flexible with regards to incoming
> > patches.  If it fixes a bug, let's take it.
> 
> I don't agree completely. While I accept that it can slowdown the
> development speed I also think that if we don't _try_ to produce
> patches for every change we do we won't get a good unit testing.
> 
> Writting tests take time and I'm sure we all are busy people who lack
> the need time to work at Parted and other projects, if we don't try to
> keep them up they'll just die and then no useful tests will be written
> since development has to be done.

I don't think anyone is disagreeing with this plan.  My point is that
right now the changes we are making to parted are either really tiny and
virtually unnoticable -or- they are huge rewrites of a lot of bad code.
If a developer can contribute a good patch, but does not provide a test
case, we're not throwing out that patch if it works.  Someone here can
write the test case.

I may be missing something, but was there a problem with you writing the
test case if Jim had submitted the patch (forget for the moment that the
commit came before patch review on the mailing list....assume that
happened)?

> >> Besides, there's a bad indentation on the commited patch, see:
> >
> > This is a different problem entirely.  Did we all ever agree on an
> > indentation style?  We discussed it, but I don't remember a standard
> > being agreed upon.
> >
> > I want spaces rather than tabs, 8 spaces per indentation level.  Tabs
> > used in Makefiles for obvious reasons.  No more than 80 chars per line.
> 
> Sure it's a different problem but we need to define a standard for it.
> 
> I'd still want to write an indent script to format the whole tree code
> and check against mistakes but I also think we _need_ to define the
> code style before I can do it.

Right, the script comes after the definition.

> I personally dislike the 8 spaces and I'm starting to think that tabs
> might grant the visual flexibility we might need. Most of time 2 or 4
> spaces are enough and makes easier to avoid line breaks.

Tabs now?  I thought we were all in the spaces camp.

I'll say my personal preference is 4 spaces for indentation, never tabs
in C.  I use tabs in Python code.

-- 
David Cantrell <dcantrell at redhat.com>
Red Hat / Westford, MA
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.alioth.debian.org/pipermail/parted-devel/attachments/20070518/f1689382/attachment.pgp