← Back to team overview

ubuntu-touch-coreapps-reviewers team mailing list archive

Re: [Merge] lp:~vthompson/music-app/decode-before-ms2-lookup into lp:music-app

 

Review: Needs Fixing

Not added in your mp but in app/components/Helpers/UriHandlerHelper.qml we double decode the URI could you remove the instance on line 71. Otherwise this mp looks good :-)

app/components/Helpers/UriHandlerHelper.qml:71:        uri = decodeURIComponent(uri);
app/components/Helpers/UriHandlerHelper.qml:74:        var track = musicStore.lookup(decodeURIComponent(uri));

Also grep'ing reveals alot of the time we do if (ms.lookup(decode(path)) { something = ms.lookup(decode(path)) } I wonder how long the lookup takes and if it would be worthwhile storing in a var so that we don't double lookup everything or if it is actually really quick and not worth it, that's probably for another mp though ;-)


-- 
https://code.launchpad.net/~vthompson/music-app/decode-before-ms2-lookup/+merge/260891
Your team Music App Developers is subscribed to branch lp:music-app.


References