Code review comment for lp:~spiv/bzr/sprout-does-not-reopen-repo

Revision history for this message
Vincent Ladeuil (vila) wrote :

Thanks, Jelmer !

Ok, so my main concerns are addressed, just a few remarks, possibly tweaks:

379 + if not isinstance(repository, RemoteRepository):
380 + raise AssertionError('xxx %r' % (repository,))

replacing this 'xxx' with something meaningful can only help people confronted with the raised assertion ;)

406 + raise AssertionError(
407 + 'diff %r: %r vs. (%r + %r)' %
408 + (url_diff, repository.user_url, medium.base, repo_path))

 ditto (less sure here, the tb may be enough)

And finally, you summary in your reply above would be nice to keep in devnotes IMHO. Let's not redo this the next time we want to address the deeper issue.

review: Approve

« Back to merge proposal