launchpad-reviewers team mailing list archive

[Benji York <benji@xxxxxxxxxxxxx>]

Review: Needs Fixing code

Given that a common failure in testing is to neglect error conditions, I
am concerned that any errors in this transformation will sneak through a
complete test run, and since reviewing this by hand is a non-starter, I
decided to write a program to parse the diff and whitelist the
transformations that I figured were totally safe and then manually
review all the remaining changes.  These are my findings.

lib/lp/bugs/externalbugtracker/bugzilla.py contained the only bad
transformation I could find.

-        except (xmlrpclib.ResponseError, xml.parsers.expat.ExpatError):
+        except xmlrpclib.ResponseError as xml.parsers.expat.ExpatError:

There were some instances of except clauses that contained unnecessary
tupilization that was carried over in the transformation.  It would be
nice to fix these too:

=== modified file 'lib/lp/app/widgets/tests/test_datetime.py'
-        except (ValueError,), e:
+        except (ValueError,) as e:


=== modified file 'lib/lp/registry/scripts/distributionmirror_prober.py'
-        except (UnknownURLScheme,), e:
+        except (UnknownURLScheme,) as e:
-        except (InfiniteLoopDetected,), e:
+        except (InfiniteLoopDetected,) as e:

-- 
https://code.launchpad.net/~cjwatson/launchpad/new-style-except/+merge/112727
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.