Merge lp://staging/~benji/launchpad/bug-597324 into lp://staging/launchpad
Proposed by
Benji York
| Status: | Merged |
|---|---|
| Approved by: | Māris Fogels |
| Approved revision: | no longer in the source branch. |
| Merged at revision: | 11378 |
| Proposed branch: | lp://staging/~benji/launchpad/bug-597324 |
| Merge into: | lp://staging/launchpad |
| Diff against target: |
368 lines (+184/-102) 4 files modified
lib/canonical/launchpad/webapp/login.py (+0/-9) lib/canonical/launchpad/webapp/publication.py (+85/-72) lib/canonical/launchpad/webapp/tests/test_login.py (+0/-18) lib/canonical/launchpad/webapp/tests/test_publication.py (+99/-3) |
| To merge this branch: | bzr merge lp://staging/~benji/launchpad/bug-597324 |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Māris Fogels (community) | Approve | ||
|
Review via email:
|
|||
Description of the change
This merge proposal covers three relatively small changes to this branch
(r11114-r11118):
- removal an attempted (and ill-fated) work-around for bug 608920
- addition of tests for all existing maybe_block_
paths (in preparation to add another exception)
- addition of referer requirement exception for OpenID callback (fixes
the error described in
https:/
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
Hi Benji,
This change looks good to me. The test suite addition is particularly welcome. That code is absolutely critical judging by the number and detail of the comments, so the additional test coverage counts many many times over. Big +1 for that.
On line 352 of the diff you have a comment that says '# this should not raise an exception', and then on the next line you call self.assertRais
Otherwise, this change looks good. r=mars
Maris