Subject: Re: [PATCH] microproject for GSOC

From: Matthieu Moy <[email-masked]>
Date: Fri, 14 Mar 2014 18:10:57 +0100

Hi,

Welcome to the Git community, and welcome to the GSOC program. Below are
some comments to give you a taste of what a review looks like on this
list. Do take the comments seriously (they should be addressed), but
don't take them badly: critic is meant to be constructive.

ubuntu733 <[email-masked]> writes:
^^^^^^^^^

Please, use a real name when you contribute to Git.

> Apply for GSOC.The microprojects is rewriter diff-index.c

This part of your message will become the commit message (i.e. cast in
stone forever in git.git's history). The point is not that you want to
apply for GSOC, but what the patch does and more importantly why it does
it.

> +#define REMOVE 1

If the code is to be removed, then remove it. That's why we use a
version control system ;-).

> -	while ((e = readdir(dir)))
> -		if (strcmp(".", e->d_name) && strcmp("..", e->d_name))
> -			string_list_insert(list, e->d_name);
> -
> +	while ((e = readdir(dir))) {
> +		while(is_dot_or_dotdot(e->d_name))

Missing space between "while" and "(".

> +                      break;

Broken indentation (indent with space).

This while (...) break; seems really weird to me: if the condition is
false, then you exit the loop because it's a while loop, and if the
condition is true, you exit the loop because of the break. Isn't that a
no-op?

> +		string_list_insert(list, e->d_name);
> +              }

Broken indentation (misplaced }).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/