Re: [PATCH 06/21] Refactor tag name verification loop to use index 'i' instead of incrementing pointer 'tag_line'

Hi,

On Sat, 9 Jun 2007, Johan Herland wrote:

On Saturday 09 June 2007, Alex Riesen wrote:

quoted

On 6/9/07, Johan Herland [off-list ref] wrote:

quoted

Signed-off-by: Johan Herland <redacted>
---
 mktag.c |   29 ++++++++++++++++-------------
 1 files changed, 16 insertions(+), 13 deletions(-)

What is this change good for?
How did you justify the type selection for your
loop index variable?

IOW,  the patch looks very useless.

I agree. By itself, the patch is useless.

Then it shouldn't be there.

It seems that you do not place the cuts between patches at the 
_conceptual_ layer. Therefore, they seem intrusive and often the meaning 
evades me.

So, if I understood the purpose of this patch series correctly, namely to 
use the same verification routines both for creation as for validation of 
tags, you could have

	- moved one function into the library (the stricter one), saying 
	  "move this_function() into libgit.a to make it usable from 
	   git-bla" in the commit body,

	- used that from the other program, removing the now-unused 
	  function,

	- and then changed the behaviour to be more chatty or some such.

As it is, you have a mix of conceptually different changes in almost every 
patch, and some changes that conceptually belong into the same patch, are 
not.

Be that as may, I think it is not a good change to reuse the same function 
like you did, exactly because one version _should_ be more forgiving than 
the other.

Ciao,
Dscho