Code review comment for lp:~mbp/bzr/scripts

Revision history for this message
Martin Pool (mbp) wrote :

On 15 September 2010 03:29, Vincent Ladeuil <email address hidden> wrote:
> Review: Needs Fixing
> I agree, having the test writer think about -q and '...' sounds like a good plan.
> The only drawback I could think of is that it will require a bit more expertise
> than a mere copy/paste. No big loss here, I'm still waiting for bug reporters to
> do that anyway :-D
>
> 127     - test_case.assertEqualDiff(expected, actual)
> 128     + test_case.assertEquals(expected, actual)
>
> Why ??? EqualDiff provides valuable info, please don't remove that !

The diff is nicer to read, but it's less precise when there are
differences of whitespace etc, to the extent that I couldn't work out
what was going wrong. Perhaps I'll write a separate method that tries
to get the best of both, in a followon patch. For now I'll just
revert that.

--
Martin

« Back to merge proposal