Code review comment for lp:~james-w/linaro-image-tools/tarfile-improvements

On Tue, 31 Aug 2010 10:48:06 -0400, James Westby <email address hidden> wrote:
> On Tue, 31 Aug 2010 01:13:40 -0000, Michael Hudson <email address hidden> wrote:
> > The standard_tarfile context manager is only used in tests, so maybe
> > it should be defined in the test module.
>
> It's used in two test modules later, which is why I moved it here, but
> I'll move it to a testing module (if it stays given your comments on the
> next proposal.)

Moved.

> > I don't like the way that the tests "know" that create_simple_tarball
> > creates a file called foo with content bar. I think it would be
> > better to use a pattern like bzrlib's build_tree where you say
> > create_simple_tarball([('foo', 'bar')]) or something.
>
> I think that's a good idea. I had them all explicitly creating the
> paths, but given that they all created identical paths I just factored
> it out.

Made the change as suggested. I don't like the duplication with the
helper I add in the next branch, but I'm not sure I like indirecting
through a helper in order to test the functionality.

Thanks,

James