On 03/23/2012 02:39 PM, Junio C Hamano wrote:
ericc<eric.chamberland@xxxxxxxxxxxxxxx> writes:
Actually, doing a 'cvs -n commit' will _do_ the commit...
With this patch, it now goes through the code, but don't do the commit.
OK.
A further progress would be to do the pre-commit hook is possible...
Sorry, I wanted to write:
"A further progress would be to do the pre-commit hook *if* possible..."
here, we are used to do "cvs -n commit" just to check if the "hooks" on
the cvs server will fail or not...
I understand that you tried to make the patch smaller by avoiding
re-indenting, but this is *yucky*.
It looks to me that the above part could be solved with:
unless (...) {
next;
}
I think the function being patched is too big. Wouldn't it be better to
have a refactoring patch to move the above per-path logic to a helper
function that deals with a single path, and then insert the "omit call to
that helper when run with -n" code in a separate patch?
The same comment applies to the other hunk.
Also I notice that the indentation used throughout the file is somewhat
broken (e.g. "Emulate by running hooks/update" part is indented to 8
columns, but earlier parts use 4 space indent). The right structure for
this change may be:
Patch 1: Fix indentation (and do nothing else) to uniformly indent with
HT;
Patch 2: Refactor this big funciton using a handful of helper functions
(and do nothing else);
Patch 3: Omit calls to these helper functions under -n option.
Ok you are right... These were my very first lines in Perl... I just
wanted to catch the attention of someone who is able to do the changes
correctly... and in a more clean way than I...
Eric
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html