Sorry I've been so long in responding to this. I was tasked on other projects. I've finally been able to context switch back to git-ubuntu and hope to be on it for the next week or so.
I've made a bunch of comments inline. I'm sorry I didn't notice the kwargs thing before; I realise that was present in my original review.
In any case, given that it's been so long, I don't think it's fair to ask you to take all of this on again. So I've prepared a branch that addresses my comments, which I can post as a separate MP and supersede this MP if you're happy with that as an approach to proceeding?
In that branch I've also written a proposed replacement for your test_importer_main_cleanup_on_exception called test_importer_close_repository_on_exception. I've left test_importer_main_cleanup_on_exception there for now, but I propose (weakly) that it can be dropped because test_importer_close_repository_on_exception is simpler and combined with your unit tests on GitUbuntuRepository.close() establish the same regression check that test_importer_main_cleanup_on_exception does.
Sorry I've been so long in responding to this. I was tasked on other projects. I've finally been able to context switch back to git-ubuntu and hope to be on it for the next week or so.
I've made a bunch of comments inline. I'm sorry I didn't notice the kwargs thing before; I realise that was present in my original review.
In any case, given that it's been so long, I don't think it's fair to ask you to take all of this on again. So I've prepared a branch that addresses my comments, which I can post as a separate MP and supersede this MP if you're happy with that as an approach to proceeding?
In that branch I've also written a proposed replacement for your test_importer_ main_cleanup_ on_exception called test_importer_ close_repositor y_on_exception. I've left test_importer_ main_cleanup_ on_exception there for now, but I propose (weakly) that it can be dropped because test_importer_ close_repositor y_on_exception is simpler and combined with your unit tests on GitUbuntuReposi tory.close( ) establish the same regression check that test_importer_ main_cleanup_ on_exception does.
I've pushed my branch to https:/ /code.launchpad .net/~racb/ usd-importer/ +git/usd- importer/ +ref/importer- cleanup- tmpdir- always - feel free to resubmit this MP using that branch if you'd like, or I can do it, or we can find some other way forward.