<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="https://git.reviewboard.kde.org/r/116017/">https://git.reviewboard.kde.org/r/116017/</a>
     </td>
    </tr>
   </table>
   <br />










<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On February 24th, 2014, 7:17 p.m. UTC, <b>David Faure</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  



<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
 <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
    <a href="https://git.reviewboard.kde.org/r/116017/diff/1/?file=245840#file245840line307" style="color: black; font-weight: bold; text-decoration: underline;">kio/kio/job.h</a>
    <span style="font-weight: normal;">

     (Diff revision 1)

    </span>
   </th>
  </tr>
 </thead>

 <tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
  <tr>

   <td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">namespace KIO {</pre></td>

  </tr>
 </tbody>



 
 

 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
    <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">307</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm">     * you use the function for when doing HTTP POST.</span></pre></td>
  </tr>

 </tbody>

</table>

  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">"recommended you use the function for when..." - rather unclear. Which function? In the commit log you said you wanted to encourage using the QIODevice overload directly (which I assume this is trying to do, although not clearly).

But even then I don't think it makes sense. Anyone who has a QByteArray on their hands should call the function with a QByteArray rather than just create a QBuffer in order to be able to call the "recommended function" -- all this achieves is duplicating that bit of code into the apps.

I would suggest removing the whole note, simply. Two overloads = pick the most convenient one for the calling code.
</pre>
 </blockquote>



 <p>On February 25th, 2014, 5:14 a.m. UTC, <b>Dawit Alemayehu</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Well the intent was to discourage people from using QByteArrays to store upload data in the first place so as to avoid the same mistake committed in KHTML. Currently KHTML stores the contents of an entire file, no matter how large, into memory whenever a user attempts to upload such files. My attempt to get the KHTML developers to realize that is the wrong thing to do did not get anywhere. Anyways, you are correct. It is pointless to attempt to discourage people from using API that has been available since the creation of KIO.</pre>
 </blockquote>







</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Oh, I agree. If one can provide the data as a QIODevice that retrieves data on demand that is indeed much better.

But simply deprecating the method that takes a QByteArray will lead to absurdity for people who actually have a QByteArray in the first place and no other choice: they'd have to create a QBuffer around it just to call the right method, that's just inconvenient.

However I agree - if one has a choice, one should prefer not to create a qbytearray with the full data in the first place.

"has been available since the creation of kio" isn't at all any reason for or against, times change and things improve :) We deprecate things for a good reason. I just don't think there's a good reason to deprecate the QByteArray overload. At most it can say in its documentation "if you can, try not to load the full data upfront, and use the QIODevice overload instead".</pre>
<br />




<p>- David</p>


<br />
<p>On February 27th, 2014, 1:41 p.m. UTC, Dawit Alemayehu wrote:</p>








<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for kdelibs, Andrea Iacovitti and David Faure.</div>
<div>By Dawit Alemayehu.</div>


<p style="color: grey;"><i>Updated Feb. 27, 2014, 1:41 p.m.</i></p>







<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>


 <a href="http://bugs.kde.org/show_bug.cgi?id=330890">330890</a>


</div>



<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
kdelibs
</div>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
 <table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The attached patch implements support for redirecting one POST request to another in KIO. Unless implicitly disabled currently the automatic redirection handler in KIO always redirects any POST requests to a GET.

Note this patch also changes the original KIO::http_post implementation that accepted a QByteArray to simply store the data in a QBuffer and call the newer implementation that uses a QIODevice. I have updated the documentation for the original implementation to state as such and encourage developers to directly use the newer http_post method instead. Not sure if everyone will agree with my implementation but it makes it much easier to resend POST data on redirection.
</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">http://greenbytes.de/tech/tc/httpredirects/t307methods.html
http://greenbytes.de/tech/tc/httpredirects/t308methods.html
http://www.w3.org/People/Bos/Test/redirect307.html</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>kioslave/http/http.cpp <span style="color: grey">(9eba5d1)</span></li>

 <li>kio/kio/job_p.h <span style="color: grey">(5ead3ed)</span></li>

 <li>kio/kio/job.h <span style="color: grey">(aeaffa2)</span></li>

 <li>kio/kio/job.cpp <span style="color: grey">(edc5fed)</span></li>

</ul>

<p><a href="https://git.reviewboard.kde.org/r/116017/diff/" style="margin-left: 3em;">View Diff</a></p>







  </td>
 </tr>
</table>








  </div>
 </body>
</html>