Merge lp://qastaging/~allenap/launchpad/refactor-get-email-notifications into lp://qastaging/launchpad

Proposed by Gavin Panella
Status: Merged
Approved by: Brad Crittenden
Approved revision: no longer in the source branch.
Merged at revision: 11405
Proposed branch: lp://qastaging/~allenap/launchpad/refactor-get-email-notifications
Merge into: lp://qastaging/launchpad
Diff against target: 411 lines (+274/-60)
2 files modified
lib/lp/bugs/scripts/bugnotification.py (+49/-53)
lib/lp/bugs/scripts/tests/test_bugnotification.py (+225/-7)
To merge this branch: bzr merge lp://qastaging/~allenap/launchpad/refactor-get-email-notifications
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+32922@code.qastaging.launchpad.net

Commit message

Refactor get_email_notifications().

Description of the change

This refactors the get_email_notifications() to make sense :) I was working in the same file and got a little carried away with fixing it, so split it off into a separate branch.

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

This is a nice fix Gavin. The new code is much easier to understand than what you're replacing. Thanks.

review: Approve (code)
Revision history for this message
Deryck Hodge (deryck) wrote :

Hey look, there's Gavin. :) First, hurrah for making this better and also for breaking it into smaller, land-able chunks!

The unit test seems to cover a fair bit of batching notifications ground. Are the existing tests that can be removed now?

Cheers,
deryck

Revision history for this message
Gavin Panella (allenap) wrote :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.