Re: GSOC idea: build in scripts and cleanups

From: Robert David <robert.david.public@gmail.com>
Date: Apr 8, 2011, 09:51 UTC
Message-ID: <201104081151.23392.robert.david.public@gmail.com>
In-Reply-To: <7vk4f5emcm.fsf@alter.siamese.dyndns.org>

Dne pátek 08 dubna 2011 00:19:05 Junio C Hamano napsal(a):

Show 12 quoted lines

> Robert David <robert.david.public@gmail.com> 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?

Show 16 quoted lines

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

Show 8 quoted lines

> 
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html