[Merge][#301969]Give proper error message for diff with non-existent dotted revno
John Arbash Meinel
john at arbash-meinel.com
Mon Dec 1 18:39:11 GMT 2008
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Marius Kruger wrote:
> 2008/12/1 Aaron Bentley <aaron at aaronbentley.com
> <mailto:aaron at aaronbentley.com>>
>
> Aaron Bentley has voted resubmit.
> Status is now: Resubmit
> Comment:
> It doesn't make sense to use NULL_REVISION when the revno is not
> supplied. That is a specific revision-- it doesn't mean "unspecified".
>
> I'm sorry about that, while trying to figure out whats wrong I got a
> deprecation warning,
> since some code down the line used the null revid. Now that we give the
> proper exception I don't see that warning any more.
>
> updated patch attached.
>
> thanks a lot for looking at this
> marius
>
This seems like something that should be a direct test on
RevisionSpec_revno, rather than tested at the 'diff' level. One test
there is fine, but the detailed tests should be asserting that
errors.InvalidRevisionSpec is being raised.
I believe "len(revisions) != 1" is because at one point we had a bug in
our dotted revnos when you had ghosts, which could cause more than one
revision to have the same revno. This has since been fixed, so doing:
if not revisions:
or
if revisions == []
or something like that would be fine as well. Anyway, that was how you
could have gotten "None" as the revision_id, but still had something
that was somewhat valid.
So I think the fix is appropriate.
Anyway, I'd like to see the tests cleaned up, but as this is still an
improvement and it *is* tested I'll go ahead and merge it.
John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iEYEARECAAYFAkk0L08ACgkQJdeBCYSNAAO00wCfeifo3H3P1/AawHlaxa8P5PNx
6BQAoLInmjC5scbr/M1P0Ty07uI6i73k
=f1DD
-----END PGP SIGNATURE-----
More information about the bazaar
mailing list