D22958: Add initial Android support to ecm_add_app_icon

2019-08-06 Thread Eike Hein
hein abandoned this revision. hein added a comment. Marco explained privately he doesn't want this so I'll move it to my codebase for now until the alternative better thing appears. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D22958 To: hein, #framewo

D22958: Add initial Android support to ecm_add_app_icon

2019-08-06 Thread Eike Hein
hein added a comment. In D22958#507299 , @nicolasfella wrote: > Why do you restrict it to SVGs? Wouldn't you be able to use PNGs as well? Mostly because I don't know what assumptions Kirigami makes. Its own macro is hardcoded to only wor

D22958: Add initial Android support to ecm_add_app_icon

2019-08-06 Thread Nicolas Fella
nicolasfella added a comment. Why do you restrict it to SVGs? Wouldn't you be able to use PNGs as well? REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D22958 To: hein, #frameworks, mart, leinir, apol Cc: nicolasfella, kde-frameworks-devel, kde-buildsystem

D22958: Add initial Android support to ecm_add_app_icon

2019-08-06 Thread Marco Martin
mart requested changes to this revision. mart added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > ECMAddAppIcon.cmake:157 > +else() > +install(FILES ${icon_full} DESTINATION > ${KDE_INSTALL_QMLDIR}/org/kde/kirigami.2/icons/

D22958: Add initial Android support to ecm_add_app_icon

2019-08-05 Thread Eike Hein
hein added inline comments. INLINE COMMENTS > apol wrote in ECMAddAppIcon.cmake:154 > this if could go up to where we're checking the icon name in an elseif up at > line 146. We'd need to do a `get_filename_component` there and end up either pulling the `ext` thing out twice (unless CMake does

D22958: Add initial Android support to ecm_add_app_icon

2019-08-05 Thread Aleix Pol Gonzalez
apol added a comment. The patch makes sense. +1 INLINE COMMENTS > ECMAddAppIcon.cmake:154 > +set(ext "${CMAKE_MATCH_4}") > +if(NOT ${icon_type} STREQUAL ".svg" OR NOT ${icon_type} > STREQUAL ".svgz") > +message(AUTHOR_WARNING "${icon_full

D22958: Add initial Android support to ecm_add_app_icon

2019-08-05 Thread Eike Hein
hein added a comment. This is the culmination of a patch series that makes About dialogs in Kirigami work reasonably across platforms: F7168840: about.jpg REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D22958 T

D22958: Add initial Android support to ecm_add_app_icon

2019-08-05 Thread Eike Hein
hein added a comment. The code is mostly copy-and-pasted together from ECMAddAppIcon.cmake and ECMInstallIcons.cmake. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D22958 To: hein, #frameworks, mart, leinir, apol Cc: kde-frameworks-devel, kde-buildsyste

D22958: Add initial Android support to ecm_add_app_icon

2019-08-05 Thread Eike Hein
hein created this revision. hein added reviewers: Frameworks, mart, leinir, apol. Herald added projects: Frameworks, Build System. Herald added subscribers: kde-buildsystem, kde-frameworks-devel. hein requested review of this revision. REVISION SUMMARY Kirigami has a `kirigami_package_breeze_ico