Re: Review Request: Make KGlobalSettings reread locale settings before calling settingsChanged().
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103469/#review10141 --- This review has been submitted with commit 87756493f51716594c5ad40f29a5c92ee66e0425 by Lamarque V. Souza to branch KDE/4.8. - Commit Hook On Dec. 19, 2011, 10:15 p.m., Lamarque Vieira Souza wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103469/ --- (Updated Dec. 19, 2011, 10:15 p.m.) Review request for kdelibs and Plasma. Description --- This is patch number 3 to fix bug #289094 (top bar time incorrectly displays in 24 hour format). The other patches are against plasma-mobile and kde-workspace (http://git.reviewboard.kde.org/r/103434), all three patches must be applied to fix the bug. I think this is a much simpler solution than the one I suggested in http://git.reviewboard.kde.org/r/103434. This addresses bug 289094. http://bugs.kde.org/show_bug.cgi?id=289094 Diffs - kdecore/localization/klocale.h 3495431 kdecore/localization/klocale.cpp 499bf11 kdecore/localization/klocale_p.h 4ed8e3f kdeui/kernel/kglobalsettings.h cb8f7e2 kdeui/kernel/kglobalsettings.cpp 819b314 Diff: http://git.reviewboard.kde.org/r/103469/diff/diff Testing --- Thanks, Lamarque Vieira Souza ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Make KGlobalSettings reread locale settings before calling settingsChanged().
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103469/#review9300 --- This review has been submitted with commit 7bc79dbefc2f52f92657d87c13c73170a5313e40 by Lamarque V. Souza to branch KDE/4.8. - Commit Hook On Dec. 19, 2011, 10:15 p.m., Lamarque Vieira Souza wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103469/ --- (Updated Dec. 19, 2011, 10:15 p.m.) Review request for kdelibs and Plasma. Description --- This is patch number 3 to fix bug #289094 (top bar time incorrectly displays in 24 hour format). The other patches are against plasma-mobile and kde-workspace (http://git.reviewboard.kde.org/r/103434), all three patches must be applied to fix the bug. I think this is a much simpler solution than the one I suggested in http://git.reviewboard.kde.org/r/103434. This addresses bug 289094. http://bugs.kde.org/show_bug.cgi?id=289094 Diffs - kdecore/localization/klocale.h 3495431 kdecore/localization/klocale.cpp 499bf11 kdecore/localization/klocale_p.h 4ed8e3f kdeui/kernel/kglobalsettings.h cb8f7e2 kdeui/kernel/kglobalsettings.cpp 819b314 Diff: http://git.reviewboard.kde.org/r/103469/diff/diff Testing --- Thanks, Lamarque Vieira Souza ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Make KGlobalSettings reread locale settings before calling settingsChanged().
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103469/#review9292 --- Ship it! Looks good. You can commit to kdelibs, branch KDE/4.8 - David Faure On Dec. 19, 2011, 10:15 p.m., Lamarque Vieira Souza wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103469/ --- (Updated Dec. 19, 2011, 10:15 p.m.) Review request for kdelibs and Plasma. Description --- This is patch number 3 to fix bug #289094 (top bar time incorrectly displays in 24 hour format). The other patches are against plasma-mobile and kde-workspace (http://git.reviewboard.kde.org/r/103434), all three patches must be applied to fix the bug. I think this is a much simpler solution than the one I suggested in http://git.reviewboard.kde.org/r/103434. This addresses bug 289094. http://bugs.kde.org/show_bug.cgi?id=289094 Diffs - kdecore/localization/klocale.h 3495431 kdecore/localization/klocale.cpp 499bf11 kdecore/localization/klocale_p.h 4ed8e3f kdeui/kernel/kglobalsettings.h cb8f7e2 kdeui/kernel/kglobalsettings.cpp 819b314 Diff: http://git.reviewboard.kde.org/r/103469/diff/diff Testing --- Thanks, Lamarque Vieira Souza ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Make KGlobalSettings reread locale settings before calling settingsChanged().
Hi all, Can I submit the review below? By the way, is kdelibs still closed to pushes? Em Monday 19 December 2011, Lamarque Vieira Souza escreveu: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103469/ --- (Updated Dec. 19, 2011, 10:15 p.m.) Review request for kdelibs and Plasma. Changes --- Add KLocale::reparseConfiguration() and make KLocalePrivate::initFormat() public. Description --- This is patch number 3 to fix bug #289094 (top bar time incorrectly displays in 24 hour format). The other patches are against plasma-mobile and kde-workspace (http://git.reviewboard.kde.org/r/103434), all three patches must be applied to fix the bug. I think this is a much simpler solution than the one I suggested in http://git.reviewboard.kde.org/r/103434. This addresses bug 289094. http://bugs.kde.org/show_bug.cgi?id=289094 Diffs (updated) - kdecore/localization/klocale.h 3495431 kdecore/localization/klocale.cpp 499bf11 kdecore/localization/klocale_p.h 4ed8e3f kdeui/kernel/kglobalsettings.h cb8f7e2 kdeui/kernel/kglobalsettings.cpp 819b314 Diff: http://git.reviewboard.kde.org/r/103469/diff/diff Testing --- Thanks, Lamarque Vieira Souza -- Lamarque V. Souza KDE's Network Management maintainer http://planetkde.org/pt-br ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Make KGlobalSettings reread locale settings before calling settingsChanged().
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103469/#review9078 --- kdeui/kernel/kglobalsettings.cpp http://git.reviewboard.kde.org/r/103469/#comment7509 I like the idea (KGlobalSettings reparsing configuration), but not the implementation (setLanguage(langage)). Can't KLocale get a reparseConfiguration() (to reuse the KConfig naming) ? - David Faure On Dec. 19, 2011, 2:02 p.m., Lamarque Vieira Souza wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103469/ --- (Updated Dec. 19, 2011, 2:02 p.m.) Review request for kdelibs and Plasma. Description --- This is patch number 3 to fix bug #289094 (top bar time incorrectly displays in 24 hour format). The other patches are against plasma-mobile and kde-workspace (http://git.reviewboard.kde.org/r/103434), all three patches must be applied to fix the bug. I think this is a much simpler solution than the one I suggested in http://git.reviewboard.kde.org/r/103434. This addresses bug 289094. http://bugs.kde.org/show_bug.cgi?id=289094 Diffs - kdeui/kernel/kglobalsettings.h cb8f7e2 kdeui/kernel/kglobalsettings.cpp 819b314 Diff: http://git.reviewboard.kde.org/r/103469/diff/diff Testing --- Thanks, Lamarque Vieira Souza ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Make KGlobalSettings reread locale settings before calling settingsChanged().
On Dec. 19, 2011, 2:35 p.m., David Faure wrote: kdeui/kernel/kglobalsettings.cpp, line 886 http://git.reviewboard.kde.org/r/103469/diff/2/?file=43867#file43867line886 I like the idea (KGlobalSettings reparsing configuration), but not the implementation (setLanguage(langage)). Can't KLocale get a reparseConfiguration() (to reuse the KConfig naming) ? KLocale::reparseConfiguration() would be just a call to KLocalePrivate::initFormat(), but initFormat is protected. I will have to make it public to use it in KLocale::reparseConfiguration, wouldn't that break binary compatibility? Another alternative is creating a public KLocalePrivate::reparseConfiguration() just to call KLocalePrivate::initFormat(). - Lamarque Vieira --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103469/#review9078 --- On Dec. 19, 2011, 2:02 p.m., Lamarque Vieira Souza wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103469/ --- (Updated Dec. 19, 2011, 2:02 p.m.) Review request for kdelibs and Plasma. Description --- This is patch number 3 to fix bug #289094 (top bar time incorrectly displays in 24 hour format). The other patches are against plasma-mobile and kde-workspace (http://git.reviewboard.kde.org/r/103434), all three patches must be applied to fix the bug. I think this is a much simpler solution than the one I suggested in http://git.reviewboard.kde.org/r/103434. This addresses bug 289094. http://bugs.kde.org/show_bug.cgi?id=289094 Diffs - kdeui/kernel/kglobalsettings.h cb8f7e2 kdeui/kernel/kglobalsettings.cpp 819b314 Diff: http://git.reviewboard.kde.org/r/103469/diff/diff Testing --- Thanks, Lamarque Vieira Souza ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Make KGlobalSettings reread locale settings before calling settingsChanged().
On Monday 19 December 2011 15:46:14 Lamarque Vieira Souza wrote: On Dec. 19, 2011, 2:35 p.m., David Faure wrote: kdeui/kernel/kglobalsettings.cpp, line 886 http://git.reviewboard.kde.org/r/103469/diff/2/?file=43867#file43867lin e886 I like the idea (KGlobalSettings reparsing configuration), but not the implementation (setLanguage(langage)). Can't KLocale get a reparseConfiguration() (to reuse the KConfig naming) ? KLocale::reparseConfiguration() would be just a call to KLocalePrivate::initFormat(), but initFormat is protected. I will have to make it public to use it in KLocale::reparseConfiguration, wouldn't that break binary compatibility? Aeh? If it's called KLocalePrivate it's a private class, you can do whatever you want with it. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Sponsored by Nokia to work on KDE, incl. KDE Frameworks 5 ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Make KGlobalSettings reread locale settings before calling settingsChanged().
Em Monday 19 December 2011, David Faure escreveu: On Monday 19 December 2011 15:46:14 Lamarque Vieira Souza wrote: On Dec. 19, 2011, 2:35 p.m., David Faure wrote: kdeui/kernel/kglobalsettings.cpp, line 886 http://git.reviewboard.kde.org/r/103469/diff/2/?file=43867#file43867 lin e886 I like the idea (KGlobalSettings reparsing configuration), but not the implementation (setLanguage(langage)). Can't KLocale get a reparseConfiguration() (to reuse the KConfig naming) ? KLocale::reparseConfiguration() would be just a call to KLocalePrivate::initFormat(), but initFormat is protected. I will have to make it public to use it in KLocale::reparseConfiguration, wouldn't that break binary compatibility? Aeh? If it's called KLocalePrivate it's a private class, you can do whatever you want with it. Well, it's called KLocalePrivate but it has a lot of protected methods that KLocale cannot access, initFormat is one of them: ../../kdecore/localization/klocale_p.h: In member function 'void KLocale::reparseConfiguration()': ../../kdecore/localization/klocale_p.h:81:18: error: 'virtual void KLocalePrivate::initFormat()' is protected ../../kdecore/localization/klocale.cpp:786:19: error: within this context -- Lamarque V. Souza KDE's Network Management maintainer http://planetkde.org/pt-br ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Make KGlobalSettings reread locale settings before calling settingsChanged().
On Monday 19 December 2011 15:01:51 Lamarque V. Souza wrote: Em Monday 19 December 2011, David Faure escreveu: On Monday 19 December 2011 15:46:14 Lamarque Vieira Souza wrote: On Dec. 19, 2011, 2:35 p.m., David Faure wrote: kdeui/kernel/kglobalsettings.cpp, line 886 http://git.reviewboard.kde.org/r/103469/diff/2/?file=43867#file4386 7 lin e886 I like the idea (KGlobalSettings reparsing configuration), but not the implementation (setLanguage(langage)). Can't KLocale get a reparseConfiguration() (to reuse the KConfig naming) ? KLocale::reparseConfiguration() would be just a call to KLocalePrivate::initFormat(), but initFormat is protected. I will have to make it public to use it in KLocale::reparseConfiguration, wouldn't that break binary compatibility? Aeh? If it's called KLocalePrivate it's a private class, you can do whatever you want with it. Well, it's called KLocalePrivate but it has a lot of protected methods that KLocale cannot access, initFormat is one of them: ../../kdecore/localization/klocale_p.h: In member function 'void KLocale::reparseConfiguration()': ../../kdecore/localization/klocale_p.h:81:18: error: 'virtual void KLocalePrivate::initFormat()' is protected ../../kdecore/localization/klocale.cpp:786:19: error: within this context So make it public. As I said, it's a Private class, there's no possible BIC issue with it. Of course I trust you that calling it directly is okay, I have no idea about that, John Layt would be able to confirm that, though. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Sponsored by Nokia to work on KDE, incl. KDE Frameworks 5 ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Make KGlobalSettings reread locale settings before calling settingsChanged().
Em Monday 19 December 2011, David Faure escreveu: On Monday 19 December 2011 15:01:51 Lamarque V. Souza wrote: Em Monday 19 December 2011, David Faure escreveu: On Monday 19 December 2011 15:46:14 Lamarque Vieira Souza wrote: On Dec. 19, 2011, 2:35 p.m., David Faure wrote: kdeui/kernel/kglobalsettings.cpp, line 886 http://git.reviewboard.kde.org/r/103469/diff/2/?file=43867#file4 386 7 lin e886 I like the idea (KGlobalSettings reparsing configuration), but not the implementation (setLanguage(langage)). Can't KLocale get a reparseConfiguration() (to reuse the KConfig naming) ? KLocale::reparseConfiguration() would be just a call to KLocalePrivate::initFormat(), but initFormat is protected. I will have to make it public to use it in KLocale::reparseConfiguration, wouldn't that break binary compatibility? Aeh? If it's called KLocalePrivate it's a private class, you can do whatever you want with it. Well, it's called KLocalePrivate but it has a lot of protected methods that KLocale cannot access, initFormat is one of them: ../../kdecore/localization/klocale_p.h: In member function 'void KLocale::reparseConfiguration()': ../../kdecore/localization/klocale_p.h:81:18: error: 'virtual void KLocalePrivate::initFormat()' is protected ../../kdecore/localization/klocale.cpp:786:19: error: within this context So make it public. As I said, it's a Private class, there's no possible BIC issue with it. Of course I trust you that calling it directly is okay, I have no idea about that, John Layt would be able to confirm that, though. Well, it works here and nothing has crashed so far :-) By the number of public and protected sections in KLocalePrivate either everybody is scared to organize them or there must be a really nasty bug in there that nobody dares to talk about because there is no comment explaining why it is like that. PS: There are 14 public: sections, 10 protected: and 2 private: in KLocalePrivate. -- Lamarque V. Souza KDE's Network Management maintainer http://planetkde.org/pt-br ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Make KGlobalSettings reread locale settings before calling settingsChanged().
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103469/ --- (Updated Dec. 19, 2011, 10:15 p.m.) Review request for kdelibs and Plasma. Changes --- Add KLocale::reparseConfiguration() and make KLocalePrivate::initFormat() public. Description --- This is patch number 3 to fix bug #289094 (top bar time incorrectly displays in 24 hour format). The other patches are against plasma-mobile and kde-workspace (http://git.reviewboard.kde.org/r/103434), all three patches must be applied to fix the bug. I think this is a much simpler solution than the one I suggested in http://git.reviewboard.kde.org/r/103434. This addresses bug 289094. http://bugs.kde.org/show_bug.cgi?id=289094 Diffs (updated) - kdecore/localization/klocale.h 3495431 kdecore/localization/klocale.cpp 499bf11 kdecore/localization/klocale_p.h 4ed8e3f kdeui/kernel/kglobalsettings.h cb8f7e2 kdeui/kernel/kglobalsettings.cpp 819b314 Diff: http://git.reviewboard.kde.org/r/103469/diff/diff Testing --- Thanks, Lamarque Vieira Souza ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
