Code review comment for lp:~mbp/bzr/764108-diffoptions

Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 8/2/2011 1:56 AM, Jelmer Vernooij wrote:
> This looks good to me.
>
> I wonder what happens if there is no external diff tool that we can
> use. Does external_diff() fail gracefully?
>
> Similarly, not all external diff tools might display the function
> name.
>
> We should probably also protect the test with a feature that checks
> if a functional external diff is available.

External diff will fail noisily. However:

 merge: approve

There is a text conflict in here that will need to be fixed before it
can land.

Martin, I thought you had created a helper for "iter_search_rules()"
that returned the 1 entry that you were most likely to want. That might
cleanup the check a bit.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk47wLQACgkQJdeBCYSNAAP3NgCffvb9PMZNes2HaT+qhYmd8YTk
Y8cAn0od51oMcxsfx1Wcn4VrGpZb13N2
=N00d
-----END PGP SIGNATURE-----

review: Approve

« Back to merge proposal