Re: Review Request 122182: [KUnitConversion] Currency: Give default values for various currencies
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122182/#review74530 --- +1 it seems better than returning nonsense even if it's slightly out of date. - David Edmundson On Jan. 21, 2015, 2:47 p.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122182/ --- (Updated Jan. 21, 2015, 2:47 p.m.) Review request for KDE Frameworks. Repository: kunitconversion Description --- The currency converter works by fetching a file from the network every day and using those conversion values. If network is not available it will try and use the previous version of that file. If no file then it will use the default values. The default values was 1e+99 in all cases. This patch updates it to the current values as reflected in the file. Diffs - src/currency.cpp 715233c Diff: https://git.reviewboard.kde.org/r/122182/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122181: [KUnitConversion] UnitCategory convert: Call UnitCategoryPrivate instead of duplicating it
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122181/#review74529 --- Ship it! Ship It! - David Edmundson On Jan. 21, 2015, 2:47 p.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122181/ --- (Updated Jan. 21, 2015, 2:47 p.m.) Review request for KDE Frameworks. Repository: kunitconversion Description --- This is probably a mistake when implementing the private class. Both UnitCategoryPrivate::convert and UnitCategory::convert essentially the same thing. However, all sub categories modify the UnitCategoryPrivate virtual method, so we should be calling that one. This was caught while trying to debug the currency converter. It has its own custom convert function. Diffs - src/unitcategory.cpp c34217e Diff: https://git.reviewboard.kde.org/r/122181/diff/ Testing --- Fixes a test in another patch. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122182: [KUnitConversion] Currency: Give default values for various currencies
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122182/#review74531 --- would it be possible to return an error if there is no useable value? We can update now, but in a month or so the values could be completely different again (c.f. EUR - CHF from two weeks ago and today). - Martin Gräßlin On Jan. 21, 2015, 3:47 p.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122182/ --- (Updated Jan. 21, 2015, 3:47 p.m.) Review request for KDE Frameworks. Repository: kunitconversion Description --- The currency converter works by fetching a file from the network every day and using those conversion values. If network is not available it will try and use the previous version of that file. If no file then it will use the default values. The default values was 1e+99 in all cases. This patch updates it to the current values as reflected in the file. Diffs - src/currency.cpp 715233c Diff: https://git.reviewboard.kde.org/r/122182/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 121903: Clean up how we deal with debug input
On Jan. 7, 2015, 6:18 p.m., David Edmundson wrote: startkde/startkde.cmake, line 12 https://git.reviewboard.kde.org/r/121903/diff/1/?file=338977#file338977line12 Order of evaluation: QtProject/qtlogging.ini setFilterRules() QT_LOGGING_CONF QT_LOGGING_RULES This will block all of the other methods from working. I don't think we can really do that. I'd prefer making a template qtlogging.ini if the file doesn't exist; as that still allows apps to override libs on/off if they prefer and allows people to set a logging_conf if they prefer. Aleix Pol Gonzalez wrote: I'm unsure how to do that. Ideas? Martin Klapetek wrote: Maybe just replace it with QLoggingCategory::setFilterRules(*.debug=false) in main? That still gives QT_LOGGING_CONF and QT_LOGGING_RULES to overwrite it. Aleix Pol Gonzalez wrote: That's what I wanted to do initially, but then with this we still only do it for plasmashell. I think it's good to have it elsewhere as well (thinking about kded mainly). Martin Gräßlin wrote: can't we ship a kconf update script to modify qtlogging.ini? Pushed to davidedmundson/defaultloggingrules my kconf_update script wasn't being run properly and I never figured out why. Shell script part is fine though. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121903/#review73399 --- On Jan. 7, 2015, 5:57 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121903/ --- (Updated Jan. 7, 2015, 5:57 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-workspace Description --- At the moment we are having a very ugly setting in plasmashell called --shut-up to filter output by default. This patch tries to address that by defining, in startkde, by default: ```QT_LOGGING_RULES=*.debug=false``` This filters out all q*Debug calls. It's better because it won't filter warnings so they can be read from .xsession-errors in case we need to debug things (at the moment it was useless because we were filtering everything) and it will also work for other processes, which can also come in handy. Developers might want to ```unset QT_LOGGING_RULES``` after this Diffs - shell/main.cpp 005f908 startkde/startkde.cmake 046543e Diff: https://git.reviewboard.kde.org/r/121903/diff/ Testing --- Log out + log in seemed to work after the changes, I'm unsure how to test, especially the startkde part. Should be quite straightforward though. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122183: [KUnitConversion] Currency: Fetch the currency file properly
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122183/#review74534 --- src/currency.cpp https://git.reviewboard.kde.org/r/122183/#comment51690 Ouch. Hello unexpected reentrancy. If this code is async, it should provide a Job API instead of masquerading under a sync API with a nasty nested event loop. Or is this running in a separate thread? - David Faure On Jan. 21, 2015, 2:47 p.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122183/ --- (Updated Jan. 21, 2015, 2:47 p.m.) Review request for KDE Frameworks. Bugs: 340819 https://bugs.kde.org/show_bug.cgi?id=340819 Repository: kunitconversion Description --- Currency: Fetch the currency file properly Properly run an event loop and wait for the file to be fetched. Also add a test to make sure currency conversion is working. This patch also contains - * https://git.reviewboard.kde.org/r/122182/ * https://git.reviewboard.kde.org/r/122181/ * https://git.reviewboard.kde.org/r/122180/ This is because reviewboard refuses to upload only a part of the diff. Please only look at currency.cpp w.r.t the EventLoop. Diffs - autotests/convertertest.h 8129a48 autotests/convertertest.cpp ae4298e src/currency.cpp 715233c src/unit.cpp 013b5d7 src/unitcategory.cpp c34217e Diff: https://git.reviewboard.kde.org/r/122183/diff/ Testing --- Test now passes. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122198: Add support for _NET_WM_OPAQUE_REGION
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122198/ --- Review request for KDE Frameworks and kwin. Repository: kwindowsystem Description --- NETWinInfo provides the opaque region as a vector of NETRect. CHANGELOG: support for _NET_WM_OPAQUE_REGION Diffs - src/netwm.cpp 600b0f6edbd42dd57f9bc2c1ce5dbc91844d344d src/netwm_def.h 3066726699947b3415433107e88a049584f3ebbc src/netwm_p.h e709caf16481938bb9f93083139ec3ea49e0bcb7 autotests/netrootinfotestwm.cpp c88ba0a10555181b8b80c9156e587185455d5047 autotests/netwininfotestclient.cpp cebf93b50a8917d243523269ecfe117f343d7f30 src/netwm.h 7729294286b3ec6dc94684ffde36b32eace7ae0d Diff: https://git.reviewboard.kde.org/r/122198/diff/ Testing --- Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122182: [KUnitConversion] Currency: Give default values for various currencies
On Jan. 22, 2015, 12:22 p.m., Martin Gräßlin wrote: would it be possible to return an error if there is no useable value? We can update now, but in a month or so the values could be completely different again (c.f. EUR - CHF from two weeks ago and today). Given the way KUnitConversion is structured it would not be a trivial change. Please note that this case will only ever occur if you use currency conversion the first time and there is no network access. The moment we have network access a more accurate currency file will be downloaded. - Vishesh --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122182/#review74531 --- On Jan. 21, 2015, 2:47 p.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122182/ --- (Updated Jan. 21, 2015, 2:47 p.m.) Review request for KDE Frameworks. Repository: kunitconversion Description --- The currency converter works by fetching a file from the network every day and using those conversion values. If network is not available it will try and use the previous version of that file. If no file then it will use the default values. The default values was 1e+99 in all cases. This patch updates it to the current values as reflected in the file. Diffs - src/currency.cpp 715233c Diff: https://git.reviewboard.kde.org/r/122182/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122183: [KUnitConversion] Currency: Fetch the currency file properly
On Jan. 22, 2015, 12:45 p.m., David Faure wrote:
src/currency.cpp, line 672
https://git.reviewboard.kde.org/r/122183/diff/1/?file=343932#file343932line672
Ouch.
Hello unexpected reentrancy.
If this code is async, it should provide a Job API instead of
masquerading under a sync API with a nasty nested event loop.
Or is this running in a separate thread?
It isn't running in a separate thread.
Hmm. This is strange. The previous code is as follows -
QNetworkReply *reply = manager.get(QNetworkRequest(QUrl(URL)));
QFile cacheFile(m_cache);
cacheFile.open(QFile::WriteOnly);
while (!reply-error() !reply-atEnd()) {
if (reply-bytesAvailable()0 || reply-waitForReadyRead(500)) {
cacheFile.write(reply-readAll());
}
}
QNetworkReply::waitForReadyRead is not implemented, so it uses
QIODeivce::waitForReadyRead which just returns false. So this code basicallly
keeps looping until we get a reply.
Possible ways to fix this -
1. Event Loop but only process some events
2. A loop as before
3. Async code where we fetch the currency rates in the background and for this
result we convert it with the previous rates.
I think (3) would be the best option. Opinions?
- Vishesh
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122183/#review74534
---
On Jan. 21, 2015, 2:47 p.m., Vishesh Handa wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122183/
---
(Updated Jan. 21, 2015, 2:47 p.m.)
Review request for KDE Frameworks.
Bugs: 340819
https://bugs.kde.org/show_bug.cgi?id=340819
Repository: kunitconversion
Description
---
Currency: Fetch the currency file properly
Properly run an event loop and wait for the file to be fetched.
Also add a test to make sure currency conversion is working.
This patch also contains -
* https://git.reviewboard.kde.org/r/122182/
* https://git.reviewboard.kde.org/r/122181/
* https://git.reviewboard.kde.org/r/122180/
This is because reviewboard refuses to upload only a part of the diff. Please
only look at currency.cpp w.r.t the EventLoop.
Diffs
-
autotests/convertertest.h 8129a48
autotests/convertertest.cpp ae4298e
src/currency.cpp 715233c
src/unit.cpp 013b5d7
src/unitcategory.cpp c34217e
Diff: https://git.reviewboard.kde.org/r/122183/diff/
Testing
---
Test now passes.
Thanks,
Vishesh Handa
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122204: Mark results as required.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122204/ --- Review request for KDE Frameworks and Chusslove Illich. Repository: ki18n Description --- This prevents API misuage, similar to how QString::arg is doing it. See also: http://quickgit.kde.org/?p=kdevplatform.gita=commith=078f1cd584e2de75ad19c46282b47dd1e0feff8f Diffs - src/klocalizedstring.h 8e8e84926b444150e00c80b3b77766ba16e03e25 Diff: https://git.reviewboard.kde.org/r/122204/diff/ Testing --- Thanks, Milian Wolff ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Build failed in Jenkins: kde-baseapps_master_qt5 #206
See http://build.kde.org/job/kde-baseapps_master_qt5/206/changes Changes: [stefano.crocco] Ported validators plugin to KF5 -- [...truncated 2917 lines...] [ 64%] Building CXX object dolphin/src/CMakeFiles/dolphinprivate.dir/dolphinprivate_automoc.cpp.o Linking CXX executable undomanagertest [ 64%] Built target konqhtmltest Linking CXX executable konqviewmgrtest [ 64%] Built target undomanagertest [ 64%] Built target konqviewmgrtest Linking CXX shared library libdolphinprivate.so [ 64%] Built target dolphinprivate [ 64%] [ 64%] Generating dolphin_folderspanelsettings.h, dolphin_folderspanelsettings.cpp Generating dolphin_searchsettings.h, dolphin_searchsettings.cpp [ 64%] Generating dolphin_informationpanelsettings.h, dolphin_informationpanelsettings.cpp [ 64%] Generating dolphin_placespanelsettings.h, dolphin_placespanelsettings.cpp Scanning dependencies of target kcm_dolphinnavigation [ 64%] [ 64%] Building CXX object dolphin/src/CMakeFiles/kcm_dolphinnavigation.dir/settings/kcm/kcmdolphinnavigation.cpp.o Scanning dependencies of target dolphinpart Generating dolphin_searchsettings.h, dolphin_searchsettings.cpp [ 64%] Building CXX object dolphin/src/CMakeFiles/dolphinpart.dir/dolphinpart.cpp.o Scanning dependencies of target kcm_dolphinservices Scanning dependencies of target kfileitemmodelbenchmark [ 64%] Scanning dependencies of target dolphinsearchboxtest Building CXX object dolphin/src/CMakeFiles/kcm_dolphinservices.dir/settings/kcm/kcmdolphinservices.cpp.o [ 64%] [ 64%] Building CXX object dolphin/src/tests/CMakeFiles/kfileitemmodelbenchmark.dir/kfileitemmodelbenchmark.cpp.o Building CXX object dolphin/src/tests/CMakeFiles/dolphinsearchboxtest.dir/dolphinsearchboxtest.cpp.o Scanning dependencies of target kcm_dolphingeneral Scanning dependencies of target kcm_dolphinviewmodes [ 64%] [ 64%] Building CXX object dolphin/src/CMakeFiles/kcm_dolphingeneral.dir/settings/kcm/kcmdolphingeneral.cpp.o Building CXX object dolphin/src/CMakeFiles/kcm_dolphinviewmodes.dir/settings/kcm/kcmdolphinviewmodes.cpp.o Scanning dependencies of target kfileitemlistviewtest [ 64%] Building CXX object dolphin/src/tests/CMakeFiles/kfileitemlistviewtest.dir/kfileitemlistviewtest.cpp.o [ 64%] Building CXX object dolphin/src/CMakeFiles/kcm_dolphinnavigation.dir/settings/navigation/navigationsettingspage.cpp.o [ 65%] Building CXX object dolphin/src/CMakeFiles/kcm_dolphinservices.dir/settings/services/servicessettingspage.cpp.o [ 65%] Building CXX object dolphin/src/CMakeFiles/kcm_dolphingeneral.dir/settings/general/behaviorsettingspage.cpp.o Scanning dependencies of target kdeinit_dolphin [ 65%] Building CXX object dolphin/src/CMakeFiles/kcm_dolphinviewmodes.dir/settings/viewmodes/dolphinfontrequester.cpp.o [ 66%] Building CXX object dolphin/src/CMakeFiles/kdeinit_dolphin.dir/dolphinapplication.cpp.o [ 66%] Building CXX object dolphin/src/tests/CMakeFiles/dolphinsearchboxtest.dir/__/search/dolphinfacetswidget.cpp.o [ 66%] [ 67%] Building CXX object dolphin/src/tests/CMakeFiles/kfileitemmodelbenchmark.dir/testdir.cpp.o Building CXX object dolphin/src/CMakeFiles/dolphinpart.dir/dolphinpart_ext.cpp.o [ 67%] Building CXX object dolphin/src/tests/CMakeFiles/kfileitemlistviewtest.dir/testdir.cpp.o http://build.kde.org/job/kde-baseapps_master_qt5/ws/dolphin/src/settings/navigation/navigationsettingspage.cpp: In member function ‘virtual void NavigationSettingsPage::applySettings()’: http://build.kde.org/job/kde-baseapps_master_qt5/ws/dolphin/src/settings/navigation/navigationsettingspage.cpp:91:106: warning: ‘static void KGlobalSettings::emitChange(KGlobalSettings::ChangeType, int)’ is deprecated (declared at /srv/jenkins/install/linux/x86_64/g++/kf5-qt5/frameworks/kdelibs4support/inst/include/KF5/KDELibs4Support/kglobalsettings.h:559) [-Wdeprecated-declarations] KGlobalSettings::self()-emitChange(KGlobalSettings::SettingsChanged, KGlobalSettings::SETTINGS_MOUSE); ^ http://build.kde.org/job/kde-baseapps_master_qt5/ws/dolphin/src/settings/navigation/navigationsettingspage.cpp: In member function ‘void NavigationSettingsPage::loadSettings()’: http://build.kde.org/job/kde-baseapps_master_qt5/ws/dolphin/src/settings/navigation/navigationsettingspage.cpp:115:59: warning: ‘static bool KGlobalSettings::singleClick()’ is deprecated (declared at /srv/jenkins/install/linux/x86_64/g++/kf5-qt5/frameworks/kdelibs4support/inst/include/KF5/KDELibs4Support/kglobalsettings.h:113) [-Wdeprecated-declarations] const bool singleClick = KGlobalSettings::singleClick(); ^ [ 67%] Building CXX object dolphin/src/CMakeFiles/kcm_dolphinnavigation.dir/settings/settingspagebase.cpp.o [ 67%] http://build.kde.org/job/kde-baseapps_master_qt5/ws/dolphin/src/settings/general/behaviorsettingspage.cpp: In member function
Re: Review Request 122204: Mark results as required.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122204/#review74568 --- +1 This change is not source-compatible though... 8-) or is it? - Aleix Pol Gonzalez On Jan. 22, 2015, 7:34 p.m., Milian Wolff wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122204/ --- (Updated Jan. 22, 2015, 7:34 p.m.) Review request for KDE Frameworks and Chusslove Illich. Repository: ki18n Description --- This prevents API misuage, similar to how QString::arg is doing it. See also: http://quickgit.kde.org/?p=kdevplatform.gita=commith=078f1cd584e2de75ad19c46282b47dd1e0feff8f Diffs - src/klocalizedstring.h 8e8e84926b444150e00c80b3b77766ba16e03e25 Diff: https://git.reviewboard.kde.org/r/122204/diff/ Testing --- Thanks, Milian Wolff ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122204: Mark results as required.
On Jan. 23, 2015, 12:11 a.m., Aleix Pol Gonzalez wrote: +1 This change is not source-compatible though... 8-) or is it? Milian Wolff wrote: It's _meant_ to be source-incompatible. All places where it doesn't compile are doing it wrong™. If you mean abi-incompatible, then no - I don't think this changes the mangled name and thus anything while linking. I might be wrong though. I know it's meant to do that, but we're still supposed to be source compatible. I don't think it would be very nice that there's some (broken) software that uses KF5 that can't be compiled anymore. I'm not against introducing this, BTW, I think it's a nice help, just wanted to highlight it because I think it's important to know what people would think about this. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122204/#review74568 --- On Jan. 22, 2015, 7:34 p.m., Milian Wolff wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122204/ --- (Updated Jan. 22, 2015, 7:34 p.m.) Review request for KDE Frameworks and Chusslove Illich. Repository: ki18n Description --- This prevents API misuage, similar to how QString::arg is doing it. See also: http://quickgit.kde.org/?p=kdevplatform.gita=commith=078f1cd584e2de75ad19c46282b47dd1e0feff8f Diffs - src/klocalizedstring.h 8e8e84926b444150e00c80b3b77766ba16e03e25 Diff: https://git.reviewboard.kde.org/r/122204/diff/ Testing --- Thanks, Milian Wolff ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122204: Mark results as required.
On Jan. 22, 2015, 11:11 p.m., Aleix Pol Gonzalez wrote: +1 This change is not source-compatible though... 8-) or is it? It's _meant_ to be source-incompatible. All places where it doesn't compile are doing it wrong™. If you mean abi-incompatible, then no - I don't think this changes the mangled name and thus anything while linking. I might be wrong though. - Milian --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122204/#review74568 --- On Jan. 22, 2015, 6:34 p.m., Milian Wolff wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122204/ --- (Updated Jan. 22, 2015, 6:34 p.m.) Review request for KDE Frameworks and Chusslove Illich. Repository: ki18n Description --- This prevents API misuage, similar to how QString::arg is doing it. See also: http://quickgit.kde.org/?p=kdevplatform.gita=commith=078f1cd584e2de75ad19c46282b47dd1e0feff8f Diffs - src/klocalizedstring.h 8e8e84926b444150e00c80b3b77766ba16e03e25 Diff: https://git.reviewboard.kde.org/r/122204/diff/ Testing --- Thanks, Milian Wolff ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122204: Mark results as required.
On Jan. 22, 2015, 11:11 p.m., Aleix Pol Gonzalez wrote: +1 This change is not source-compatible though... 8-) or is it? Milian Wolff wrote: It's _meant_ to be source-incompatible. All places where it doesn't compile are doing it wrong™. If you mean abi-incompatible, then no - I don't think this changes the mangled name and thus anything while linking. I might be wrong though. Aleix Pol Gonzalez wrote: I know it's meant to do that, but we're still supposed to be source compatible. I don't think it would be very nice that there's some (broken) software that uses KF5 that can't be compiled anymore. I'm not against introducing this, BTW, I think it's a nice help, just wanted to highlight it because I think it's important to know what people would think about this. This will just warn by default, though -- at least true for GCC. IMO, it's still worth it, given that using the i18n API can easily lead to no-ops in code (I've been there myself). This doesn't affect the mangling either = ABI compatible - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122204/#review74568 --- On Jan. 22, 2015, 6:34 p.m., Milian Wolff wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122204/ --- (Updated Jan. 22, 2015, 6:34 p.m.) Review request for KDE Frameworks and Chusslove Illich. Repository: ki18n Description --- This prevents API misuage, similar to how QString::arg is doing it. See also: http://quickgit.kde.org/?p=kdevplatform.gita=commith=078f1cd584e2de75ad19c46282b47dd1e0feff8f Diffs - src/klocalizedstring.h 8e8e84926b444150e00c80b3b77766ba16e03e25 Diff: https://git.reviewboard.kde.org/r/122204/diff/ Testing --- Thanks, Milian Wolff ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122206: [kio] Make tests optional
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- Review request for KDE Frameworks. Repository: kio Description --- [kio] Make tests optional Diffs - CMakeLists.txt 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- This is a small patch to CMakeLists.txt to only depend on Qt5Test if BUILD_TESTING. Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 7:48 p.m.) Review request for KDE Frameworks. Repository: kio Description (updated) --- [kio] Make tests optional This is a small patch to CMakeLists.txt to only depend on Qt5Test if BUILD_TESTING. Diffs - CMakeLists.txt 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing (updated) --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122210: KCmdLineArgs: Fix -version handling
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122210/ --- Review request for KDE Frameworks. Repository: kdelibs4support Description --- Remove the TODO and fix the i18n usage Diffs - src/kdecore/kcmdlineargs.cpp de2f829 Diff: https://git.reviewboard.kde.org/r/122210/diff/ Testing --- konsole -v is no longer ugly as hell Thanks, Albert Astals Cid ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122210: KCmdLineArgs: Fix -version handling
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122210/#review74561 --- Ship it! Ship It! - Jeremy Whiting On Jan. 22, 2015, 1:43 p.m., Albert Astals Cid wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122210/ --- (Updated Jan. 22, 2015, 1:43 p.m.) Review request for KDE Frameworks. Repository: kdelibs4support Description --- Remove the TODO and fix the i18n usage Diffs - src/kdecore/kcmdlineargs.cpp de2f829 Diff: https://git.reviewboard.kde.org/r/122210/diff/ Testing --- konsole -v is no longer ugly as hell Thanks, Albert Astals Cid ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122183: [KUnitConversion] Currency: Fetch the currency file properly
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122183/#review74555 --- (make sure that 10 consecutive requests don't start 10 background download jobs, of course - keep the networkreply pointer around to know it's currently happening) - David Faure On Jan. 21, 2015, 2:47 p.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122183/ --- (Updated Jan. 21, 2015, 2:47 p.m.) Review request for KDE Frameworks. Bugs: 340819 https://bugs.kde.org/show_bug.cgi?id=340819 Repository: kunitconversion Description --- Currency: Fetch the currency file properly Properly run an event loop and wait for the file to be fetched. Also add a test to make sure currency conversion is working. This patch also contains - * https://git.reviewboard.kde.org/r/122182/ * https://git.reviewboard.kde.org/r/122181/ * https://git.reviewboard.kde.org/r/122180/ This is because reviewboard refuses to upload only a part of the diff. Please only look at currency.cpp w.r.t the EventLoop. Diffs - autotests/convertertest.h 8129a48 autotests/convertertest.cpp ae4298e src/currency.cpp 715233c src/unit.cpp 013b5d7 src/unitcategory.cpp c34217e Diff: https://git.reviewboard.kde.org/r/122183/diff/ Testing --- Test now passes. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122210: KCmdLineArgs: Fix -version handling
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122210/ --- (Updated Jan. 22, 2015, 8:57 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kdelibs4support Description --- Remove the TODO and fix the i18n usage Diffs - src/kdecore/kcmdlineargs.cpp de2f829 Diff: https://git.reviewboard.kde.org/r/122210/diff/ Testing --- konsole -v is no longer ugly as hell Thanks, Albert Astals Cid ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
