D10135: Put built-in holiday definitions into qrc

2018-01-28 Thread Dominik Haumann
dhaumann added a comment. Ok, I can see that now ;) Thanks. REPOSITORY R175 PIM: KHolidays REVISION DETAIL https://phabricator.kde.org/D10135 To: vkrause, #frameworks, #kde_pim, mlaurent Cc: dhaumann, mlaurent, dvasin, winterz, vkrause, knauss, dvratil

D10135: Put built-in holiday definitions into qrc

2018-01-28 Thread Volker Krause
vkrause added a comment. In https://phabricator.kde.org/D10135#197280, @dhaumann wrote: > - What's missing (or I miss something) is that it could be that entries exist twice, if in resource and on disk. Since this is not the case right now, this is probably not an issue. In that

D10135: Put built-in holiday definitions into qrc

2018-01-28 Thread Dominik Haumann
dhaumann added a comment. I'm late to the game, but in general looks good. Minor comments: - What's missing (or I miss something) is that it could be that entries exist twice, if in resource and on disk. Since this is not the case right now, this is probably not an issue. - holida

D10135: Put built-in holiday definitions into qrc

2018-01-27 Thread Volker Krause
This revision was automatically updated to reflect the committed changes. Closed by commit R175:730063cb9dc1: Put built-in holiday definitions into qrc (authored by vkrause). REPOSITORY R175 PIM: KHolidays CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10135?vs=26034&id=26069 REVISI

D10135: Put built-in holiday definitions into qrc

2018-01-27 Thread Volker Krause
vkrause added inline comments. INLINE COMMENTS > mlaurent wrote in CMakeLists.txt:8 > Just curious it's not default in ECM ? I never used it. Yep, I was expecting this to be the default too, but it didn't work without explicitly enabling it. REPOSITORY R175 PIM: KHolidays BRANCH master R

D10135: Put built-in holiday definitions into qrc

2018-01-27 Thread Laurent Montel
mlaurent accepted this revision. mlaurent added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > CMakeLists.txt:8 > set(CMAKE_MODULE_PATH ${ECM_MODULE_PATH}) > +set(CMAKE_AUTORCC ON) > Just curious it's not default in ECM ? I never used it. REPOSITORY R17

D10135: Put built-in holiday definitions into qrc

2018-01-27 Thread Volker Krause
vkrause created this revision. vkrause added reviewers: Frameworks, KDE PIM. Restricted Application added a project: KDE PIM. vkrause requested review of this revision. REVISION SUMMARY The file system is still searched in the same place as before, which can be easier to use than qrc while wor