ubuntu-touch-coreapps-reviewers team mailing list archive

[Victor Thompson <victor.thompson@xxxxxxxxx>]

Review: Needs Information

I have just one inline question/comment. Otherwise this looks good. I still can get the lookup failures when the source is set. However, that might be an issue with lp:1449296.

Diff comments:

> === modified file 'app/components/Player.qml'
> --- app/components/Player.qml	2015-04-29 01:12:57 +0000
> +++ app/components/Player.qml	2015-06-03 18:56:22 +0000
> @@ -209,15 +209,17 @@
>                              obj = musicStore.lookup(source.toString())
>                          }
>  
> -                        player.currentMetaAlbum = obj.album;
> -
> -                        if (obj.art !== undefined) {  // FIXME: protect against no art property in playlists
> -                            player.currentMetaArt = obj.art;
> +                        // protect against null reponse from the lookup
> +                        if (obj !== null) {
> +                            // protect against undefined properties
> +                            player.currentMetaAlbum = obj.album || "";
> +                            player.currentMetaArt = obj.art || "";  // note playlists don't have art property

I don't quite get what playlists have to do with the metadata for a particular file being played by the player. Could you explain what this note means? Is this just to note that it's not in the DB? If so, maybe say that instead.

> +                            player.currentMetaArtist = obj.author || "";
> +                            player.currentMetaFile = obj.filename || "";
> +                            player.currentMetaTitle = obj.title || "";
> +                        } else {
> +                            console.debug("Mediascanner lookup resulted in null object", source.toString())
>                          }
> -
> -                        player.currentMetaArtist = obj.author;
> -                        player.currentMetaFile = obj.filename;
> -                        player.currentMetaTitle = obj.title;
>                      }
>  
>                      console.log("Source: " + source.toString())
> 


-- 
https://code.launchpad.net/~ahayzen/music-app/capture-errors-for-currentMeta/+merge/261012
Your team Music App Developers is subscribed to branch lp:music-app.