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

From: Thomas Rast <[email-masked]>
Date: Sat, 26 Oct 2013 10:14:21 +0200

Junio C Hamano <[email-masked]> writes:

> Thomas Rast <[email-masked]> 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.)

> 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".

>> * 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
[email-masked]