[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