On Mon, 16 Apr 2012, Marc Hoersken wrote:

>> 5 - On many places in the code you use the hardcoded numers 4096 and 2048.
>>    Why those numbers? And why not use defined names for them?
>
> There is basically no reason for these specific numbers. 4096 is the initial
> read buffer size and 2048 is the increase/decrease step size. I could
> certainly use defines for those, but in reality it might be even better to
> remove them and find a way to figure out how many data actually needs to be
> stored. I am currently freeing unneeded space after the read process
> finishes, but there might be a better solution.

I see and I appreciate your intensions. But until you (or someone else) figure
that out, I still think replacing the hardcoded numbers with the use defined
names is a better idea. That will also make it easier for volunteers to
experiment with different sizes at will and better understand if all the
occurances of the numbers have the same meaning or not.

> I am fine with sending you everything as a series for patches, sure.
> Squashing individual commits also makes sense, but generally speaking I must
> say that I am not really a fan of rewriting the history of a published git
> repository.

Please note that I didn't not ask you to rewrite the history of your published
git repository - that is an insane idea. You can easily squash the commit
series into a more limited series in a non-published branch and then send us
that patch series for merging. Although I feel I should also repeat: I'm not
saying this needs to be done as I haven't actually seen the need, I'm leaving
that to your descretion. Having a somewhat clean commit series post to the
mailing list will also make all the changes much easier to review.

And please, it is much better and easier if you work on providing small and
clean patches for us to merge one by one rather than that you continue to work
and improve your code in your own branch. It will risk making the merge and
review work harder. You can then merge with our master and continue working on
fixing things from there.

-- 
  / daniel.haxx.se

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2012-04-16