Re: Review Request: dataprotocol: simplify helper code

2012-05-06 Thread Rolf Eike Beer

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

(Updated May 6, 2012, 6:10 p.m.)


Review request for kdelibs.


Changes
---

Rebased diff against current KDE/4.8 branch.


Description
---

-add some "const" and "static"
-remove function parameters that always have the same values, use local statics
 in the function to hold these
-QChar(QLatin1Char('\0')) => QChar()
-QChar == QLatin1Char('\0') => QChar::isNull()


Diffs (updated)
-

  kio/kio/dataprotocol.cpp e614476 

Diff: http://git.reviewboard.kde.org/r/104860/diff/


Testing
---

-build whole kdelibs
-dataprotocol testcases still pass


Thanks,

Rolf Eike Beer



Re: Review Request: dataprotocol: simplify helper code

2012-05-09 Thread David Faure

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104860/#review13604
---


OK, except for const QChar &.


kio/kio/dataprotocol.cpp


Not sure a const ref is a good idea, for a QChar (which is basically a 
ushort).

E.g. QString::at() and [] return a QChar, not a const ref.



kio/kio/dataprotocol.cpp


same here


- David Faure


On May 6, 2012, 6:10 p.m., Rolf Eike Beer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104860/
> ---
> 
> (Updated May 6, 2012, 6:10 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Description
> ---
> 
> -add some "const" and "static"
> -remove function parameters that always have the same values, use local 
> statics
>  in the function to hold these
> -QChar(QLatin1Char('\0')) => QChar()
> -QChar == QLatin1Char('\0') => QChar::isNull()
> 
> 
> Diffs
> -
> 
>   kio/kio/dataprotocol.cpp e614476 
> 
> Diff: http://git.reviewboard.kde.org/r/104860/diff/
> 
> 
> Testing
> ---
> 
> -build whole kdelibs
> -dataprotocol testcases still pass
> 
> 
> Thanks,
> 
> Rolf Eike Beer
> 
>



Re: Review Request: dataprotocol: simplify helper code

2012-05-09 Thread Commit Hook

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104860/#review13611
---


This review has been submitted with commit 
279694530bfd6df3b8c5a8b31d1350bd2720a81f by Rolf Eike Beer to branch KDE/4.8.

- Commit Hook


On May 6, 2012, 6:10 p.m., Rolf Eike Beer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104860/
> ---
> 
> (Updated May 6, 2012, 6:10 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Description
> ---
> 
> -add some "const" and "static"
> -remove function parameters that always have the same values, use local 
> statics
>  in the function to hold these
> -QChar(QLatin1Char('\0')) => QChar()
> -QChar == QLatin1Char('\0') => QChar::isNull()
> 
> 
> Diffs
> -
> 
>   kio/kio/dataprotocol.cpp e614476 
> 
> Diff: http://git.reviewboard.kde.org/r/104860/diff/
> 
> 
> Testing
> ---
> 
> -build whole kdelibs
> -dataprotocol testcases still pass
> 
> 
> Thanks,
> 
> Rolf Eike Beer
> 
>