Code review comment for lp:~jtv/launchpad/bug-499405-translationtemplates-buildmanager

>> + def _checkout(self, branch_url):
>> + """Check out a source branch to generate from.
>> +
>> + The branch is checked out to the location specified by
>> + `self.branch_dir`.
>> + """
>> + command = ['/usr/bin/bzr', 'checkout', branch_url, self.branch_dir]
>> + return call(command, cwd=self.home)
>>
>> Is there a reason you're not using bzrlib? I know that the build slaves have
>> special security there, but if you have bzr there, you have bzrlib.
>
>Mainly laziness. But also, this being a corner of LP that's hard to test, I would >like to minimize the risk of silly little mistakes that might not be reported back to >me very effectively. Instead of writing my own python, I'm producing a command line >here that I can just replicate in a shell when debugging.

So there are a few issues with calling the bzr runtime instead of using bzrlib. The first is that we do not want to use the system bzr. Launchpad packages a local copy of bzr for this very purpose. The second is that we aren't guaranteed we'll have the plugins we need on the system, but we make sure they're available to bzrlib. There are also performance advantages to using bzrlib specifically. I really think that bzrlib needs to be used in this situation.

Other than that, I'm happy with your other changes.