Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-23 Thread Aleix Pol Gonzalez
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126087/ --- (Updated Nov. 23, 2015, 2:33 p.m.) Status -- This change has been ma

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-23 Thread Chusslove Illich
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126087/#review88721 --- Ship it! Ship It! - Chusslove Illich On Нов. 23, 2015, 1:5

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-23 Thread Aleix Pol Gonzalez
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126087/ --- (Updated Nov. 23, 2015, 1:52 p.m.) Review request for KDE Frameworks, Chu

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-23 Thread Chusslove Illich
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126087/#review88717 --- src/klocalizedcontext.cpp (line 76)

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-23 Thread Aleix Pol Gonzalez
> On Nov. 22, 2015, 7:51 p.m., Milian Wolff wrote: > > src/klocalizedcontext.h, line 59 > > > > > > virtual It already is, I'll mark it as override. > On Nov. 22, 2015, 7:51 p.m., Milian Wolff wrote: > > src/kl

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-23 Thread Aleix Pol Gonzalez
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126087/ --- (Updated Nov. 23, 2015, 12:54 p.m.) Review request for KDE Frameworks, Ch

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-22 Thread Milian Wolff
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126087/#review88697 --- Ship it! autotests/ki18ndeclarativetest.cpp (line 31)

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-20 Thread Aleix Pol Gonzalez
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126087/ --- (Updated Nov. 20, 2015, 3:03 p.m.) Review request for KDE Frameworks, Chu

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-20 Thread Aleix Pol Gonzalez
> On Nov. 20, 2015, 2:33 p.m., Chusslove Illich wrote: > > src/klocalizedcontext.cpp, line 87 > > > > > > How are numbers converted to strings, before being passed here? Because > > when it receives actual numeri

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-20 Thread Chusslove Illich
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126087/#review88648 --- src/klocalizedcontext.h (line 28)

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-20 Thread Aleix Pol Gonzalez
> On Nov. 19, 2015, 5:21 p.m., Marco Martin wrote: > > so let's go for that and then add the import with the singleton as well, if > > is okay for framework maintainer are fine with the qtqml dependency.. but > > they should be as the "everything in kdeclarative" situation has to be > > solved

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-19 Thread Aleix Pol Gonzalez
> On Nov. 19, 2015, 5:21 p.m., Marco Martin wrote: > > so let's go for that and then add the import with the singleton as well, if > > is okay for framework maintainer are fine with the qtqml dependency.. but > > they should be as the "everything in kdeclarative" situation has to be > > solved

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-19 Thread Marco Martin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126087/#review88612 --- so let's go for that and then add the import with the singleto

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-19 Thread Aleix Pol Gonzalez
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126087/ --- (Updated Nov. 19, 2015, 5 p.m.) Review request for KDE Frameworks, Chussl

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-18 Thread Marco Martin
> On Nov. 17, 2015, 9:58 a.m., Marco Martin wrote: > > Here's the reason I don't like it: > > In origin KDeclarative was intended to be a library that only depended from > > Qt stuff, to be at most tier 2 (and imports there were supposed to be > > qt-only as well) and frameworks applications us

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-18 Thread Marco Martin
> On Nov. 17, 2015, 10:05 a.m., Marco Martin wrote: > > a public symbol could even be spared (and in the process made possibleto > > use it from pure qml) by making an import instead that does the > > setContextObject() injection in its setupEngine (that would happen > > immediately after an i

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-18 Thread Aleix Pol Gonzalez
> On Nov. 17, 2015, 11:05 a.m., Marco Martin wrote: > > a public symbol could even be spared (and in the process made possibleto > > use it from pure qml) by making an import instead that does the > > setContextObject() injection in its setupEngine (that would happen > > immediately after an i

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-18 Thread Aleix Pol Gonzalez
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126087/ --- (Updated Nov. 18, 2015, 1:26 p.m.) Review request for KDE Frameworks, Chu

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-18 Thread Aleix Pol Gonzalez
> On Nov. 17, 2015, 9:43 a.m., Chusslove Illich wrote: > > Could you also document the usage in docs/programmers-guide.md (section > > #link_cat)? I'm not much into QML, so it would help me understand the > > implications of the usage. > > > > It seems to me other three series of calls (xi18n*

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-18 Thread Chusslove Illich
> On Нов. 17, 2015, 9:43 пре п., Chusslove Illich wrote: > > Could you also document the usage in docs/programmers-guide.md (section > > #link_cat)? I'm not much into QML, so it would help me understand the > > implications of the usage. > > > > It seems to me other three series of calls (xi18

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Kai Uwe Broulik
> On Nov. 16, 2015, 2:08 nachm., Milian Wolff wrote: > > src/klocalizedcontext.cpp, line 53 > > > > > > isNull or isEmpty? Don't we usually want to check against isEmpty and > > never against isNull? Isn't the la

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Aleix Pol Gonzalez
> On Nov. 17, 2015, 10:58 a.m., Marco Martin wrote: > > Here's the reason I don't like it: > > In origin KDeclarative was intended to be a library that only depended from > > Qt stuff, to be at most tier 2 (and imports there were supposed to be > > qt-only as well) and frameworks applications u

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Aleix Pol Gonzalez
> On Nov. 17, 2015, 9:43 a.m., Chusslove Illich wrote: > > Could you also document the usage in docs/programmers-guide.md (section > > #link_cat)? I'm not much into QML, so it would help me understand the > > implications of the usage. > > > > It seems to me other three series of calls (xi18n*

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Marco Martin
> On Nov. 17, 2015, 9:58 a.m., Marco Martin wrote: > > Here's the reason I don't like it: > > In origin KDeclarative was intended to be a library that only depended from > > Qt stuff, to be at most tier 2 (and imports there were supposed to be > > qt-only as well) and frameworks applications us

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Aleix Pol Gonzalez
> On Nov. 16, 2015, 3:08 p.m., Milian Wolff wrote: > > src/klocalizedcontext.cpp, line 53 > > > > > > isNull or isEmpty? Don't we usually want to check against isEmpty and > > never against isNull? Isn't the latt

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Marco Martin
> On Nov. 17, 2015, 9:58 a.m., Marco Martin wrote: > > Here's the reason I don't like it: > > In origin KDeclarative was intended to be a library that only depended from > > Qt stuff, to be at most tier 2 (and imports there were supposed to be > > qt-only as well) and frameworks applications us

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Marco Martin
> On Nov. 17, 2015, 9:58 a.m., Marco Martin wrote: > > Here's the reason I don't like it: > > In origin KDeclarative was intended to be a library that only depended from > > Qt stuff, to be at most tier 2 (and imports there were supposed to be > > qt-only as well) and frameworks applications us

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Aleix Pol Gonzalez
> On Nov. 17, 2015, 10:58 a.m., Marco Martin wrote: > > Here's the reason I don't like it: > > In origin KDeclarative was intended to be a library that only depended from > > Qt stuff, to be at most tier 2 (and imports there were supposed to be > > qt-only as well) and frameworks applications u

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Martin Gräßlin
> On Nov. 17, 2015, 10:58 a.m., Marco Martin wrote: > > Here's the reason I don't like it: > > In origin KDeclarative was intended to be a library that only depended from > > Qt stuff, to be at most tier 2 (and imports there were supposed to be > > qt-only as well) and frameworks applications u

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Marco Martin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126087/#review88465 --- a public symbol could even be spared (and in the process made

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Marco Martin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126087/#review88463 --- Here's the reason I don't like it: In origin KDeclarative was

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Chusslove Illich
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126087/#review88459 --- Could you also document the usage in docs/programmers-guide.md

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-16 Thread Aleix Pol Gonzalez
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126087/ --- (Updated Nov. 16, 2015, 3:55 p.m.) Review request for KDE Frameworks, Chu

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-16 Thread Milian Wolff
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126087/#review88431 --- src/klocalizedcontext.h (line 63)

Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-16 Thread Andreas Cord-Landwehr
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126087/#review88428 --- I really like this change! But shouldn't there be a unit test

Review Request 126087: Move the i18n context from KDeclarative

2015-11-16 Thread Aleix Pol Gonzalez
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126087/ --- Review request for KDE Frameworks, Chusslove Illich and Marco Martin. Rep