Subject: Re: GSOC idea: build in scripts and cleanups

From: Robert David <[email-masked]>
Date: Fri, 8 Apr 2011 11:51:22 +0200

Dne pátek 08 dubna 2011 00:19:05 Junio C Hamano napsal(a):
> Robert David <[email-masked]> writes:
> > This task also include cleaning the functionality of this code, to make
> > these functions more "standardized".
> > This means consolidate the differences in these functions and make them
> > more consistent in the user point of view.
> 
> Standardized, consolidate, consistent are all pretty words but at the same
> time they are highly subjective. Even though you may not have to have the
> detailed description of everything in the proposal, you would need to at
> least have a handful of examples of what are wrong with the current code
> in mind when making such a sweeping statement, but I cannot sense that
> from this proposal.

While I have read the code, first thing I noticed is the inconsistence of 
general coding standarts used. Sometimes the code is pretty nice divided into 
subrutines to get the code understand fast, and sometimes it is a huge 
subrutine that does as much things as possible, and thus is no so good to read 
(also I don't speak here about comments missing in a lot of code, where 
sometimes there are some). With the lack of documentation in Documentation 
directory, that means that this code is something like black box. And hard to 
learn for newcomer. Tell me if Im wrong, but isnt the open-source demand also 
to let as many people ac possible to involve in development? 

The biggest example is the "patch_update_file". Which can be divided in few 
parts. For example the "if ($line) { ..... }" big big box.
This also mean for list_and_chose and list_modified (and maybe others).

Sometimes there are perl like statements:

"something if|for|while|etc something"
and common things are written next like 
"if (something) { something }"

Is there demand to write scripts (mainly perl) like in C to keep the code more 
readable for C people or just write it more in perl style (to keep the code 
cleaner and shorter)?

Another thing is parsing command line options, why not use Getopt::Long here?

> 
> While reading the current add-i code recently, I noticed that over time
> the code quality deteriorated compared to the reasonably clean design in
> the original version, perhaps due to excessive copying and mimicing,
> especially when "Edit patch" feature was added and then more recently when
> new modes to apply patches in different directions are added to allow
> other commands like "git reset" and "git checkout" to implement their
> "--patch" options.
> 
> An example of a clean-up in the right direction may be a patch I posted a
> few days ago:
> 
>  
> http://thread.gmane.org/gmane.comp.version-control.git/170685/focus=171017
> 
> Is that the kind of "consolidation" you have in mind?

I have read that conversation before. I also tried to reproduce that bug, but 
have no success (in version 1.7.4.3). But before I read the code and try to 
find something that this can occur, you send the patch. I'm not so fast as 
you:)

> 
> Also I am a bit puzzled by the inconsistency between the various stages of
> 
> your goals.  A clearly stated primary goal is:
> > Main and final project goal is integrating fully git-add--interactive
> > into current git-add code.
> 
> but the success criteria is only about the above "This task also include"?

I will try to clear that up today, to finalize that in the meaning of doing the 
cleanup and extension part as part of GSoC and prepare the rewriting part for 
further work. I still think I can handle that and rewrite that as part of 
GSoC, but I didn't do such a big open-source project before, so I'm going to 
adopt your style of coding standards and timings. 

I also attach a example perl code I'm now writing, it is a half done(40%) data 
-> animated graph converter (part of my seminar work). 

Robert.

> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to [email-masked]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html