Code review comment for lp:~frankban/python-fixtures/fakelogger-getdetails

> There seem to be a lot of whitespace changes in the diff? Possibly
> harmless, but confusing.

My editor is set up to strip spaces at the end of lines when I save a file.
I can revert changes in fixture.py if you like, I should have mentioned this earlier, sorry.

> Rather than implementing getDetails, the normal mechanism is to call
> self.addDetail with a detail that will return the appropriate content
> when evaluated. This is cheaper than evaluating when getDetails is
> called, and means you only need to test that an appropriate detail is
> added, rather than that the getDetails method still Does The Right
> Thing. The UTF8_TEXT helper is probably revelant here too, for
> brevity.

Great! I've changed the branch following this suggestion, and deleted a test.

> Finally, on the naming of things, I'd describe the attachment - think
> of it as an email attachment.

Now the detail name is an instance attribute: if you don't like this, I can just
format the string in setUp and find another way (e.g. .values()[0]) to get the
content in the test.

> Note the use of a closure here so that d = f.getDetails(),
> f.cleanUp(), f.setUp(), d.items()[0].itertext() will not return the
> new (empty) state of the fixture.

Yes, I was using a closure in the previous revision too. It's cool and, AFAICT,
lazy evaluation is the only option here, since details are gathered early in useFixture:
http://bazaar.launchpad.net/~testtools-committers/testtools/trunk/view/head:/testtools/testcase.py#L585

Thanks Robert.