Re: [PATCH] Make 'cvs -n commit ...' not to commit

> ericc<eric.chamberland@giref.ulaval.ca>  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...

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