Code review comment for lp:~michael.nelson/launchpad/567922-binarypackagebuild-packagebuild-2

Discussed on IRC. A fair portion of my notes were on docstrings that aren't new and have been handed down from times immemorial; might as well seize the opportunity to clear things up while we're here.

lib/lp/buildmaster/interfaces/buildfarmjob.py: makeJob

I thought this would be a problem with TranslationTemplatesBuildJob, which is based on BranchJob whose constructor likes to create its own Jobs. Turns out this has been taken into account, but the docstring has been clarified. Thanks!

lib/lp/buildmaster/interfaces/packagebuild.py: getUploaderCommand

Clarified: what does "the command to run as the uploader" mean? That all this is about on command, and it is to be run under the uploader's identity?

lib/lp/buildmaster/interfaces/packagebuild.py: getUploadLeaf

Docstring clarified.

The slave build id has been replaced with the slave build cookie. It is now a hash without (I hope) any distinguishable components.

lib/lp/buildmaster/tests/test_buildbase.py: BuildBaseTestCase

Don't derive base class for tests from TestCase. It's not a test. Make it a mixin instead, and use multiple inheritance in the leaf classes. Also saves you from deriving a test case from both TestCase *and* TestCaseWithFactory. Rename setUp so as not to confuse super().

At the time of writing, this is being resolved.

Good luck with this refactoring!

Jeroen