[Freeciv-Dev] Re: Documentation, Usability and Development

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Thu, Nov 29, 2001 at 05:42:49PM -0500, Justin Moore wrote:
> 
> > >    Yes, but in some cases they're the *only* way to do something.  If I'm
> > > ripping a 300 line function out of one file and creating a new one with
> > > that function in it, there is *no way* I can do that in less than 600
> > > lines.
> > If you are ripping a 300 line function, you should break it into pieces, as 
> > 300
> > may be a little too much (obviously depends on situation). And including it 
> > in
> > new one which will result in 600 lines of code looks even worse, this is 
> > imho
> > definitively a bad idea ;-).  I would make first patch which would rip the
> > function, second patch which would rip another function and third patch 
> > which
> > would create function merging those two by calling ripped bits of them ;-).
> 
>    But in even ripping apart the 300 line function, you're getting a huge
> patch.  We need a branch (not a tree, a branch) where large patches are
> acceptable PROVIDED they're simply moving code around or reworking ONE
> major concept/layout.  I think huge patches that touch 17 functions are
> horrendous and should not be accepted.  But if a patch is 300 consecutive
> lines starting with "-" and another 300 lines starting with "-" and
> everything after the "-"s and "+"s is identical, it makes no sense to
> reject it for being too large.

Ack. There was such case where code was only moved around. In the end
I have done the code movement by myself and compared it to the
patch. Far easier than to check the 600 lines patch. So such cases
aren't a problem.

> > Obviously, there are some situations when making little patch is impossible.
> > However even if the patch is larger, it should always focus only on one
> > feature, not fixing/adding more things at once.
> 
>    Correct.  The other problem I ran into which lead to huge patches was
> moving the declaration/initialization of the commands struct in
> server/stdinhand.c from one file to another.  There's no way you can break
> that up or rip it apart.  It's going in/out as a whole.  And when I
> submitted it, it (of course) was rejected as being "too large" despite the
> inherient size issues over which there is *no* control.

Too large was only one of the reasons. It had the same problem as
Andrew's patch. It wanted to make the code more extendable but it was
not clear in which direction this extendibility was needed. 

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "SIGDANGER - The System is likely to crash soon"