Code review comment for lp://qastaging/~thisfred/desktopcouch/split-attempt-2

Revision history for this message
dobey (dobey) wrote :

1408 except gnomekeyring.NoKeyringDaemonError:
1409 - logging.warn("There is no keyring to store our admin credentials.")
1410 + logging.warn(
1411 + "There is no keyring to store our admin credentials.")
1412 except gnomekeyring.CancelledError:
1413 - logging.warn("There is no keyring to store our admin credentials.")
1414 + logging.warn(
1415 + "There is no keyring to store our admin credentials.")
1416 except gnomekeyring.IOError:
1417 - logging.warn("There is no keyring to store our admin credentials.")
1418 + logging.warn(
1419 + "There is no keyring to store our admin credentials.")

These should be a single exception clause, rather than repeating the same string N times. If there is intent on providing additional information based on exception time, we should do that instead of just duplicating messages.

2011 except gnomekeyring.NoKeyringDaemonError:
2012 - logging.warn("There is no keyring to store our admin credentials.")
2013 + pass
2014 + ## logging.warn(
2015 + ## "There is no keyring to store our admin credentials.")
2016 except gnomekeyring.CancelledError:
2017 - logging.warn("There is no keyring to store our admin credentials.")
2018 + pass
2019 + ## logging.warn(
2020 + ## "There is no keyring to store our admin credentials.")

2042 except gnomekeyring.NoKeyringDaemonError:
2043 # in this case we do not have a major issue, continue and return
2044 - logging.warn("There is no keyring to store our oauth credentials.")
2045 + pass
2046 + ## logging.warn(
2047 + ## "There is no keyring to store our oauth credentials.")
2048 +
2049 except gnomekeyring.CancelledError:
2050 # in this case we do not have a major issue, continue and return
2051 - logging.warn("There is no keyring to store our admin credentials.")
2052 + pass
2053 + ## logging.warn(
2054 + ## "There is no keyring to store our admin credentials.")

These two sections of code in the tests should also be compressed into single except clauses I think, and should not just comment out code, but either import the logging module or just remove the code.

review: Needs Fixing

« Back to merge proposal