Merge lp://staging/~dylanmccall/update-manager/refactored-cellareapackage into lp://staging/update-manager

Proposed by Dylan McCall
Status: Merged
Approved by: Barry Warsaw
Approved revision: 2589
Merged at revision: 2588
Proposed branch: lp://staging/~dylanmccall/update-manager/refactored-cellareapackage
Merge into: lp://staging/update-manager
Diff against target: 142 lines (+63/-49)
1 file modified
UpdateManager/UpdatesAvailable.py (+63/-49)
To merge this branch: bzr merge lp://staging/~dylanmccall/update-manager/refactored-cellareapackage
Reviewer Review Type Date Requested Status
Barry Warsaw (community) Approve
Review via email: mp+145040@code.staging.launchpad.net

Description of the change

While playing with the package column (and pkg_area) in UpdatesAvailable.py, I noticed I was needing to perform shotgun surgery in order to make some small changes: there was a bunch of code in CellAreaPackage that depended on specifics of UpdatesAvailable's tree view. This branch contains a more generalized CellAreaPackage. It isn't exactly _portable_ at this stage, but I think it's a step in the right direction. The assumptions it continues to make are documented, and innocent changes in other parts of the program are less likely to break it. The output should be identical to the existing cell area.

To post a comment you must log in.
2589. By Dylan McCall

In CellAreaPackage, fixed indent for cells at depth 3 to be consistent with current results.

Revision history for this message
Barry Warsaw (barry) wrote :

A few comments:

Line 51: s/not cell_number in/cell_number not in/

Line 104: I factored out the "len(cells) - 1" into a local variable outside the loop for readability and (possibly) improved performance.

Lines 105-108 can be better written as:
    cell_size = self.cached_cell_size.get(cell_number, 0)

Line 110: Removed unnecessary parens.

I'll fix those and sponsor this into trunk (and upload) along with your LP: #1105363 fix.

review: Approve
Revision history for this message
Dylan McCall (dylanmccall) wrote :

Great! Thanks, Barry :)
(Somehow got all this way without noticing dict.get can return a default value. Exciting!)

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.

Subscribers

People subscribed via source and target branches

to status/vote changes: