ubuntu-touch-coreapps-reviewers team mailing list archive

[Andrew Hayzen <ahayzen@xxxxxxxxx>]

Review: Needs Fixing

Thanks for this!

One inline comment so far, also I am about to bump the version in the changelog (as we just had a release) once this is done could you pull and add an entry for yourself in there :-)

Diff comments:

> === modified file 'CMakeLists.txt'
> --- CMakeLists.txt	2015-08-08 18:32:22 +0000
> +++ CMakeLists.txt	2015-12-02 19:28:55 +0000
> @@ -60,10 +60,12 @@
>  
>  file(GLOB SRC_FILES
>       RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}
> -     *.qml *.js *.png *.js *.json)
> +     *.qml *.js *.png *.js)

Not sure why we have a duplicate *.js but why are you removing the *.json ? Can you change this to
*.qml *.js *.png *.json
And if you have A->Z OCD like me then please alphabetise them :-)

>  install(DIRECTORY app DESTINATION ${DATA_DIR})
>  install(FILES ${SRC_FILES} ${ICON_FILE} DESTINATION ${DATA_DIR})
>  
> +install(FILES music-app-content.json DESTINATION ${CMAKE_INSTALL_PREFIX}/share/content-hub/peers/ RENAME music-app)
> +
>  configure_file(${DESKTOP_FILE}.in.in ${DESKTOP_FILE}.in)
>  
>  add_custom_target(${DESKTOP_FILE} ALL


-- 
https://code.launchpad.net/~ken-vandine/music-app/content-hub-desktop/+merge/279333
Your team Music App Developers is subscribed to branch lp:music-app.