Merge lp://staging/~unity-team/unity-lens-video/split-online-results into lp://staging/unity-lens-video/remote-videos-scope-trunk

Proposed by Michal Hruby
Status: Merged
Approved by: Jim Hodapp
Approved revision: 59
Merged at revision: 58
Proposed branch: lp://staging/~unity-team/unity-lens-video/split-online-results
Merge into: lp://staging/unity-lens-video/remote-videos-scope-trunk
Diff against target: 150 lines (+61/-35)
1 file modified
src/unity-scope-video-remote (+61/-35)
To merge this branch: bzr merge lp://staging/~unity-team/unity-lens-video/split-online-results
Reviewer Review Type Date Requested Status
Jim Hodapp (community) code Approve
Review via email: mp+124371@code.staging.launchpad.net

This proposal supersedes a proposal from 2012-09-14.

Commit message

Add 'split' parameter to the query

Description of the change

Update to match latest designs - the scope now adds the 'split' parameter to the query to distinguish "online" and "suggestion" results.

To post a comment you must log in.
Revision history for this message
Jim Hodapp (jhodapp) wrote :

Line 83: Add a comment describing what the "True" is there for. This will be convenient for those reading the code who are not familiar with the scope's source structure.

It is not obvious to me how the online vs. suggestion results differ and why they need to be "split." A comment above line 89 explaining the split and how it gets used would be very helpful.

Otherwise looks great, nice work.

review: Needs Fixing (code)
Revision history for this message
Jim Hodapp (jhodapp) wrote :

Since this did operate correctly and only needed comment improvements (and since Michal is on vacation), I'm setting this to be approved.

review: Approve (code)

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 all changes: