Re: GSoC 2014: Summary so far, discussion starter: how to improve?

From: Thomas Rast <tr@thomasrast.ch>
Date: Oct 26, 2013, 08:14 UTC
Message-ID: <87txg4qwiq.fsf@linux-k42r.v.cablecom.net>
In-Reply-To: <xmqqli1lro08.fsf@gitster.dls.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 8 quoted lines

> Thomas Rast <tr@thomasrast.ch> writes:
>
>> Theories
>> ========
>>
>> * Scope creep: projects tend to get blocked on some bigger
>>   refactoring/restructuring task that was not in the original
>>   proposal.

(Full disclosure: I actually proposed this theory.)

Show 6 quoted lines

> I think that is a sign that the original proposal did not look
> enough at the existing code, dreaming of a pie-in-the-sky shiny
> features in a green-field setting. What needs to be done within the
> constraint of the existing code (including a total rewrite, if
> necessary, while keeping the project's codebase maintainable is part
> of the healthy develpment.

Hmm, yes, but it's also the only objection that I believe I have never heard, as opposed to ignored.

I'm okay if we just file this under "things to consider during project proposal review".

Show 13 quoted lines

>> * Have students review some patches
>
> I am not sure if this would help.
>
> Reviewing the patches to find style violations and off-by-one errors
> is relatively easy as it can be done with knowledge on a narrow
> isolated part of the system. Reviewing the design to make sure that
> the change fits the way how existing subsystems work, ranging from
> the internal API implementation level to consistency a changed
> behaviour is presented at the UI level, however, needs understanding
> of the far wider entire project than only the parts of the system
> the proposed change updates. It will be even more true if the chosen
> topic is a cool/shiny one.

I'd choose the middle path: review for code readability. What do the functions do? Are the functions and variables named after their roles? Is there anything that I cannot understand, and therefore warrants a comment?

That is much more difficult than just reviewing for style, while it can (usually) be done without too much knowledge of the outside.

-- 
Thomas Rast
tr@thomasrast.ch