All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Rast <tr@thomasrast.ch>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: GSoC 2014: Summary so far, discussion starter: how to improve?
Date: Sat, 26 Oct 2013 10:14:21 +0200	[thread overview]
Message-ID: <87txg4qwiq.fsf@linux-k42r.v.cablecom.net> (raw)
In-Reply-To: <xmqqli1lro08.fsf@gitster.dls.corp.google.com> (Junio C. Hamano's message of "Tue, 22 Oct 2013 14:31:35 -0700")

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

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

> 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
tr@thomasrast.ch

  reply	other threads:[~2013-10-26  8:14 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-19  6:09 GSoC 2014: Summary so far, discussion starter: how to improve? Thomas Rast
2013-10-19 10:41 ` Christian Couder
2013-10-19 12:00 ` Felipe Contreras
2013-10-19 21:51 ` Fredrik Gustafsson
2013-10-26  8:21   ` Thomas Rast
2013-10-20 10:00 ` Thomas Gummerer
2013-10-22 21:31 ` Junio C Hamano
2013-10-26  8:14   ` Thomas Rast [this message]
2013-11-21  8:36 ` Thomas Rast
2013-11-21  9:43   ` Christian Couder
2013-11-21 10:04   ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87txg4qwiq.fsf@linux-k42r.v.cablecom.net \
    --to=tr@thomasrast.ch \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.