Code review comment for lp:~stub/launchpad/mock-swift

Revision history for this message
William Grant (wgrant) wrote :

154 +import os
155 +import sys
156 +import logging
157 +
158 +from OpenSSL import crypto

os, sys and crypto are unused.

133 + def startup(self):
134 + self.setUp()
135 +
136 + def shutdown(self):
137 + self.cleanUp()
138 + while self._hasDaemonStarted():
139 + time.sleep(0.1)

Are these only here for the shutdown test? I'm not sure it's necessarily valuable to have them factored out, but it might also be worth pushing them up to TacTestSetup.

264 === modified file 'setup.py'

There should only be two new deps here; why are the indirect ones included directly? I imagine it might be for the setuptools-git pre_requires mess, but it's probably better to patch that out, as it's dreadfully fragile when there's no network access, and a security hole when there is network access.

review: Needs Fixing (code)

« Back to merge proposal