← Back to team overview

ubuntu-touch-coreapps-reviewers team mailing list archive

Re: [Merge] lp:~gang65/ubuntu-calculator-app/ubuntu-calculator-app-desktop-install-fix into lp:ubuntu-calculator-app

 

Thanks for comments.
I will try apply your remarks today evening.

I explained my intention in diff comments below.

Diff comments:

> === modified file 'CMakeLists.txt'
> --- CMakeLists.txt	2015-04-14 16:42:56 +0000
> +++ CMakeLists.txt	2015-06-23 23:33:27 +0000
> @@ -27,13 +27,12 @@
>  
>  include(GNUInstallDirs)
>  
> -set(APP_NAME                        ubuntu-calculator-app)
> +set(APP_NAME                        calculator-app)
>  set(APP_HARDCODE                    ubuntu-calculator-app)
>  set(AUTOPILOT_DIR                   ubuntu_calculator_app)
>  set(MAIN_QML                        ${APP_HARDCODE}.qml)
> -set(ICON                            "${APP_HARDCODE}.png")
> +set(ICON_FILE                       "${APP_HARDCODE}.png")
>  set(DESKTOP_FILE                    "${APP_HARDCODE}.desktop")
> -set(AUTOPILOT_DIR                   ubuntu_calculator_app)

It is duplicate set. The AUTOPILOT_DIR was set twice, that's why one "set" was removed.

>  set(APP_VERSION                     2.0)
>  
>  if(CLICK_MODE)
> @@ -41,8 +40,7 @@
>    set(UBUNTU-CALCULATOR-APP_DIR    "${CMAKE_INSTALL_DATADIR}/qml")
>  
>    set(QT_IMPORTS_DIR "${CMAKE_INSTALL_LIBDIR}")
> -  set(EXEC "qmlscene $@ ${UBUNTU-CALCULATOR-APP_DIR}/${MAIN_QML}")
> -  set(MODULE_PATH ${QT_IMPORTS_DIR})
> +  set(EXEC "qmlscene -qt5 $@ ${UBUNTU-CALCULATOR-APP_DIR}/${MAIN_QML}")
>    if(NOT BZR_REVNO)
>      execute_process(
>        COMMAND bzr revno
> @@ -76,8 +74,14 @@
>    configure_file(${UBUNTU_MANIFEST_PATH} ${CMAKE_CURRENT_BINARY_DIR}/manifest.json)
>    install(FILES ${CMAKE_CURRENT_BINARY_DIR}/manifest.json DESTINATION ${CMAKE_INSTALL_PREFIX})
>    install(FILES "${APP_HARDCODE}.apparmor" DESTINATION ${CMAKE_INSTALL_PREFIX})
> +  set(ICON ${ICON_FILE})
>  else(CLICK_MODE)
> -  set(EXEC "qmlscene $@ -I ${MODULE_PATH}  ${CMAKE_INSTALL_PREFIX}/${UBUNTU-CALCULATOR-APP_DIR}/${MAIN_QML}")
> +  set(ICON ${CMAKE_INSTALL_PREFIX}/${UBUNTU-CALCULATOR-APP_DIR}/${ICON_FILE})
> +  set(EXEC ${APP_NAME})
> +  configure_file(app/${APP_NAME}.in

My intention was to be able to run calculator-app from command line. In this MR, you could just type "calculator-app" from terminal, and you could run calculator.
In my opinion it is very useful feature. Without that you will need to run "qmlscene -qt5 <main qml file>"

> +    ${CMAKE_CURRENT_BINARY_DIR}/${APP_NAME})
> +  install(PROGRAMS ${CMAKE_CURRENT_BINARY_DIR}/${APP_NAME}
> +    DESTINATION ${CMAKE_INSTALL_BINDIR})
>  endif(CLICK_MODE)
>  
>  file(GLOB_RECURSE I18N_SRC_FILES
> 
> === modified file 'app/CMakeLists.txt'
> --- app/CMakeLists.txt	2015-04-14 16:42:56 +0000
> +++ app/CMakeLists.txt	2015-06-23 23:33:27 +0000
> @@ -3,12 +3,8 @@
>    add_custom_target(ubuntu-calculator-app_QMlFiles ALL SOURCES ${QML_JS_FILES})
>  endif(NOT "${CMAKE_CURRENT_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_BINARY_DIR}")
>  
> -if(CLICK_MODE)
> -  set(ICON ${ICON})
> -  install(FILES graphics/${ICON} DESTINATION ${CMAKE_INSTALL_PREFIX})
> -endif(CLICK_MODE)
> -
> -install(FILES ${MAIN_QML} DESTINATION ${UBUNTU-CALCULATOR-APP_DIR})
> +install(FILES graphics/${ICON_FILE} DESTINATION ${CMAKE_INSTALL_PREFIX}/${UBUNTU-CALCULATOR-APP_DIR})
> +install(FILES ${MAIN_QML} DESTINATION ${CMAKE_INSTALL_PREFIX}/${UBUNTU-CALCULATOR-APP_DIR})
>  
>  add_subdirectory(engine)
>  add_subdirectory(graphics)
> 
> === modified file 'app/graphics/CMakeLists.txt'
> --- app/graphics/CMakeLists.txt	2015-03-11 09:13:18 +0000
> +++ app/graphics/CMakeLists.txt	2015-06-23 23:33:27 +0000
> @@ -1,4 +1,7 @@
> -file(GLOB GRAPHICS_FILES *.svg)
> +set(GRAPHICS_FILES 
> +  select_none.svg  
> +  select.svg
> +)
>  

According to CMake documentation:
http://www.cmake.org/cmake/help/v3.0/command/file.html

"We do not recommend using GLOB to collect a list of source files from your source tree. If no CMakeLists.txt file changes when a source is added or removed then the generated build system cannot know when to ask CMake to regenerate."

I will remove that from this MR.

>  install(FILES ${GRAPHICS_FILES} DESTINATION ${UBUNTU-CALCULATOR-APP_DIR}/graphics)
>  
> 
> === removed file 'app/graphics/toolbarIcon@xxxxx'
> Binary files app/graphics/toolbarIcon@xxxxx	2014-11-10 09:28:27 +0000 and app/graphics/toolbarIcon@xxxxx	1970-01-01 00:00:00 +0000 differ

Ok

> === modified file 'debian/changelog'
> --- debian/changelog	2014-12-11 11:09:09 +0000
> +++ debian/changelog	2015-06-23 23:33:27 +0000
> @@ -1,3 +1,45 @@
> +ubuntu-calculator-app (2.0.197) vivid; urgency=medium
> +
> +  [ Bartosz Kosiorek ]
> +  * Do not allow swiping in Landscape mode. 
> +    Fixes: https://bugs.launchpad.net/bugs/1466635.
> +  * Fix keyboard shortcuts (eg. Ctrl+C). 
> +    Fixes: https://bugs.launchpad.net/bugs/1466634.
> +  * Add additional comments to logaritm and modulo 
> +    to avoid confusion during translation. 
> +    Fixes: https://bugs.launchpad.net/bugs/1465297.
> +  * Fix display issue. 
> +    Fixes: https://bugs.launchpad.net/bugs/1465996.
> +
> +  [ Nicholas Skaggs ]
> +  * Fix testability for screen components.
> +
> +  [ Niklas Wenzel ]
> +  * Use longer swipe in autopilot tests for revealing the scientific keyboard. 
> +    Fixes: https://bugs.launchpad.net/bugs/1464571.
> +  * Fix typing special characters on hardware keyboards. 
> +    Fixes: https://bugs.launchpad.net/bugs/1449037.
> +
> +  [ Victor Thompson ]
> +  * Change favorites translatable string. 
> +    Fixes: https://bugs.launchpad.net/bugs/1464463.
> +
> + -- Bartosz Kosiorek <gang65@xxxxxxxxxxxxxx>  Fri, 24 Jul 2015 12:00:05 +0100
> +
> +ubuntu-calculator-app (2.0.182) vivid; urgency=medium
> +  * Add haptic feedback to buttons
> +  * Update math.js library. http://pad.lv/1439846
> +  * Enable 'bug number' support to fix floating point math. 
> +    http://pad.lv/1445152
> +  * Modify splash screen. http://pad.lv/1377638
> +  * Fix keyboard focus and disappearing cursor bug. http://pad.lv/1431548
> +  * Fix directory layout for AP tests. http://pad.lv/1443680
> +  * Modified layout to fit design
> +  * Fix issue with building which corrupted translations
> +  * Updated translations
> +
> + -- Bartosz Kosiorek <gang65@xxxxxxxxxxxxxx>  Thu, 15 Jun 2015 12:00:05 +0100
> +
>  ubuntu-calculator-app (0.1) vivid; urgency=low
>  
>    * Initial release
> 


-- 
https://code.launchpad.net/~gang65/ubuntu-calculator-app/ubuntu-calculator-app-desktop-install-fix/+merge/262799
Your team Ubuntu Calculator Developers is subscribed to branch lp:ubuntu-calculator-app.


References