Code review comment for lp:~mrazik/cupstream2distro-config/jenkins-per-project

Revision history for this message
Francis Ginther (fginther) wrote :

I would prefer to see
63 + credentials = load_jenkins_credentials(
64 + os.path.expanduser(self.credentialsfile),
65 + jenkins_name)
66 + if not credentials:
67 + logging.error(
68 + 'Credentials for "{}" not found.'.format(jenkins_name))
69 + return None
kept inside of __call__ and store the parsed credentials as 'self.credentials'.

This will cause a bad credentials file to fail the script before any stack processing is done. Should also make testing easier as you can use a pre-made credentials dictionary instead of patching load_jenkins_credentials.

Otherwise I like this change.

review: Needs Fixing

« Back to merge proposal