Re: [Bug-wget] Fwd: [GSoC] Extend concurrency support in Wget

[Darshit Shah]

I have a few comments about your proposal.

1. When downloading multiple files from multiple servers, there is
also the use case where you may download multiple files while each is
downloaded using multiple threads. Think about this:

I have a 2 files I'm trying to download, both are 4 GB each. I'd
rather download them with 4 jobs each using 8 different servers than
as 2 jobs in all. It is a case which should otherwise be covered int
he implementation, but it should be there in your proposal too.

Think about how this might be implemented, and how this *might* differ
from the other cases.

2. You state re-implement Metalink 3.0 code as the first point in your
timeline. So, during this period will you not be looking at Metalink
4.0 compatibility at all? If so, when will you be looking at Metalink
4? Just mention it somewhere.

3. It looks slightly off when the dates on your timeline are not
continuous. Don't put any breaks there. You can mention Weekend, if
you want, but it should be documented. It's just easier to read when
the dates flow continuously.

4. If you have exams, relax for that period. Take some time off. Don't
mess up both your code and your exams. I suggest you concentrate on
your exams during that time.

5. You give yourself 5 days to merge code and write documentation.
That's not happening. Trust me. The merging process will take a lot
more time. You're going to be writing a lot of code that edits lots of
files in mailine. This code will not be simple to merge directly.
Also, documentation takes time.

There's also some other higher level comments about your timeline and work flow:

You wish to implement/fix Metalink first. But Metalink requires
concurrent downloading capability.
You may want to rethink the order in which you're implementing these features.

On Wed, Mar 19, 2014 at 7:40 PM, Jure Grabnar <address@hidden> wrote:
> Hi,
>
> my proposal is available on
> http://www.google-melange.com/gsoc/proposal/public/google/gsoc2014/toomanysecrets/5629499534213120
> I'm looking forward to your review.
>
> Regards,
>
>
> Jure Grabnar
>
>
>
> On 18 March 2014 01:06, Darshit Shah <address@hidden> wrote:
>>
>> Hi Jure,
>>
>> Thanks for your patches. However, I do have a few comments about the same:
>>
>> 1. Trailing Whitespaces: This is essentially extra whitespaces at the
>> end of a line or on a blank line. See [1] and [2] for more
>> information.
>> 2. The indentation is mostly right, but sometimes off.
>> 3. Your first patch is missing a ChangeLog. Every commit must be
>> accompanied by a ChangeLog entry, no matter how trivial it is.
>> 4. Your 2nd patch seems to revert things from the first one. This
>> usually means some cleanup is needed.
>>
>> I'm not completely sure of some of the details of the lines you change
>> in your second patch, but they seem a little sketchy. I'll have to dig
>> into the code and check it out.
>>
>> Also, for a non-trivial (>10 LoC) patch, you'll first need to submit
>> your copyright assignment to the FSF.
>> Giuseppe will arrange for the documents as soon as your patch is ready.
>>
>> The code however, does fix a segfault and maybe a few compiler
>> warnings. When it fixes something, an explanation is usually a nice
>> idea.
>>
>> [1]
>> http://codeimpossible.com/2012/04/02/Trailing-whitespace-is-evil-Don-t-commit-evil-into-your-repo-/
>> [2]
>> https://stackoverflow.com/questions/1583406/why-does-git-care-about-trailing-whitespace-in-my-files
>>
>> On Mon, Mar 17, 2014 at 6:56 PM, Jure Grabnar <address@hidden> wrote:
>> > Hi,
>> >
>> > this patch fixes some of compiler warnings. I was uncertain for the
>> > remaining ones (5) - I believe some of them might be stubs for upcoming
>> > features.
>> >
>> >  Best Regards,
>> >
>> >
>> > Jure Grabnar (toomanysecrets)
>>
>>
>>
>> --
>> Thanking You,
>> Darshit Shah
>
>



-- 
Thanking You,
Darshit Shah