Merge lp:~nskaggs/music-app/prevent-incorrect-no-music-screen into lp:music-app/trusty

Proposed by Nicholas Skaggs on 2014-08-07
Status: Merged
Approved by: Nicholas Skaggs on 2014-08-07
Approved revision: 559
Merged at revision: 559
Proposed branch: lp:~nskaggs/music-app/prevent-incorrect-no-music-screen
Merge into: lp:music-app/trusty
Diff against target: 23 lines (+3/-1)
1 file modified
music-app.qml (+3/-1)
To merge this branch: bzr merge lp:~nskaggs/music-app/prevent-incorrect-no-music-screen
Reviewer Review Type Date Requested Status
Ubuntu Phone Apps Jenkins Bot continuous-integration 2014-08-07 Approve on 2014-08-07
Nicholas Skaggs (community) Approve on 2014-08-07
Victor Thompson 2014-08-07 Pending
Andrew Hayzen 2014-08-07 Pending
Review via email: mp+229944@code.launchpad.net

This proposal supersedes a proposal from 2014-07-31.

Commit message

Wait until model is populated before deciding there is no music. There was a new prototype to the Songs model added that provides a "filled" signal that we should listen to in order to determine the model is full.

Description of the change

Wait until model is populated before deciding there is no music. There was a new prototype to the Songs model added that provides a "filled" signal that we should listen to in order to determine the model is full.

To post a comment you must log in.
Andrew Hayzen (ahayzen) wrote : Posted in a previous version of this proposal

Looks good so far, just wonder if onFilled would be better if it works? See inline diff comments.

review: Needs Information
Victor Thompson (vthompson) wrote : Posted in a previous version of this proposal

That should work and is more readable. I'll fix this tonight.

review: Needs Fixing
Victor Thompson (vthompson) : Posted in a previous version of this proposal
review: Abstain
Andrew Hayzen (ahayzen) wrote : Posted in a previous version of this proposal

LGTM

review: Approve
review: Approve (continuous-integration)
Nicholas Skaggs (nskaggs) wrote :

I'm unsure why we are seeing,

bzrlib.errors.UnknownErrorFromSmartServer: Server sent an unexpected error: ('error', 'GhostRevisionsHaveNoRevno', 'Could not determine revno for {<email address hidden>} because its ancestry shows a ghost at {<email address hidden>}')

so I recreated the branch and MP with hopefully a clean history this time.

Nicholas Skaggs (nskaggs) wrote :

Top approving based on previous top approval.

review: Approve
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'music-app.qml'
2--- music-app.qml 2014-08-07 01:56:33 +0000
3+++ music-app.qml 2014-08-07 14:25:56 +0000
4@@ -498,8 +498,10 @@
5 }
6
7 SongsModel {
8+ property bool populated: false
9 id: allSongsModel
10 store: musicStore
11+ onFilled: populated = true
12 }
13
14 SongsModel {
15@@ -909,7 +911,7 @@
16 title: i18n.tr("Music")
17 visible: false
18
19- property bool noMusic: allSongsModel.rowCount === 0 && loadedUI
20+ property bool noMusic: allSongsModel.rowCount === 0 && allSongsModel.populated && loadedUI
21
22 onNoMusicChanged: {
23 if (noMusic)

Subscribers

People subscribed via source and target branches

to status/vote changes: