Merge lp://staging/~rvb/launchpad/branches-timeout-bug-827935-3 into lp://staging/launchpad
| Status: | Merged | ||||
|---|---|---|---|---|---|
| Approved by: | Raphaël Badin | ||||
| Approved revision: | no longer in the source branch. | ||||
| Merged at revision: | 14254 | ||||
| Proposed branch: | lp://staging/~rvb/launchpad/branches-timeout-bug-827935-3 | ||||
| Merge into: | lp://staging/launchpad | ||||
| Diff against target: |
60 lines (+13/-7) 2 files modified
lib/lp/code/browser/branchlisting.py (+11/-5) lib/lp/code/templates/person-branches.pt (+2/-2) |
||||
| To merge this branch: | bzr merge lp://staging/~rvb/launchpad/branches-timeout-bug-827935-3 | ||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Abel Deuring (community) | Approve | ||
|
Review via email:
|
|||
Commit message
[r=adeuring][bug=827935] Try to save a query on the branch listing page by using the batch size.
Description of the change
This branch is a small optimization that will (in some cases) save a query for the https:/
The page displays a batch of a filtered collection (the filter is on branches' statuses) of branches. We want to display a text if the filtered batch is empty but other branches might be displayed if the listing was not filtered. Obviously, if the batch itself is not empty, we don't need to check the size of the whole displayable collection to know that.
Drive-by fix: remove an ill-defined (and used) function that I introduced in a previous branch…
= Tests =
This should not change existing behavior.
(14:10:17) adeuring: rvba: the changes look good, but a suggestion nevertheless: What about making is_branch_
(14:12:01) rvba: adeuring: well, the method is definitely no expensive: because self.branches(
(14:12:08) rvba: s/no/not/
(14:12:34) adeuring: rvba: ah, ok, so let's leave the @property
(14:12:48) adeuring: rvba: another question: If branch_count needs to be evaluated in is_branchcount_
(14:13:23) adeuring: ...if not, would it make sense to call resulstset.any() instead?
(14:14:51) rvba: adeuring: I'm not sure I understand your suggestion, this branch does not bring any improvement to the cases where branch_count needs to be evaluated.
(14:15:54) rvba: adeuring: When the batch is empty that is. But hopefully this wont happen in many cases because when the number of branches is huge, chances are that you have all the statuses represented and the only filtering possible is by status.
(14:17:17) adeuring: rvba: right; i I just wondered if it would be hard to add that case too. But that's probably my bad habit to squeeze too much changes into one branch ;)
(14:17:22) adeuring: rbva: r=me then