Re: Review Request 122204: Mark results as required.

2015-01-25 Thread Milian Wolff

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122204/
---

(Updated Jan. 25, 2015, 3:53 p.m.)


Status
--

This change has been marked as submitted.


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.

2015-01-23 Thread Chusslove Illich

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122204/#review74632
---

Ship it!


Ship It!

- Chusslove Illich


On Јан. 22, 2015, 7:34 по п., Milian Wolff wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122204/
 ---
 
 (Updated Јан. 22, 2015, 7:34 по п.)
 
 
 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.

2015-01-23 Thread David Faure

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122204/#review74576
---

Ship it!


It if just warns, then sure. I assume there's a -Werror= to make it error 
if desired? Would be good to do a full kdesrc-build with that set ;)

- David Faure


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 122204: Mark results as required.

2015-01-22 Thread Milian Wolff

---
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


Re: Review Request 122204: Mark results as required.

2015-01-22 Thread Aleix Pol Gonzalez

---
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.

2015-01-22 Thread Aleix Pol Gonzalez


 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.

2015-01-22 Thread Milian Wolff


 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.

2015-01-22 Thread Kevin Funk


 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