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

[Justin Moore <justin@xxxxxxxxxxx>]

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

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

-jdm

Department of Computer Science, Duke University, Durham, NC 27708-0129
Email:  justin@xxxxxxxxxxx