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

[Petr Baudis <pasky@xxxxxxxxxxx>]

> > > > Huge patches are bad idea, approval chance near zero, IMHO.
> > > 
> > > Yeah, but *WHY*?
> > Because it is hard to review those patches. And if something will go wrong,
> > and the patch will need to be reverted, it will get reverted as a whole,
> > not only a part. Higher count of smaller patches are always better
> > definitively.
> 
> So tell me then: what's the difference between reverting the entire patch and
> not accepting it at all?  In terms of the final effect on the code, there is
> *no* difference.  And yet, that's exactly the comparison you're making.
If you will read it once more, you will maybe recognize that I don't compare
them, but I'm putting an equal sign between them.

> But if you accept the patch, there is a chance (perhaps a good one, even)
> that it won't need to be reverted, and the program will be improved as a
> result.
For a first sight, then random mysterious errors appearing, some design issues
devasted etc. You can't accept everything blindly.  It must not contain basic
design leaks and evident errors relying that something uninitialized will be
zero etc. This may work now, and crash later, and it is not very easy to track
it down then.

You may be amazed, but software is not only a lot of lines of code, which put
together compiles and works, but also a mean behind that, called 'design'. And
if some software has to be extensible and allow further development, it has to
abide some basic design.  If you will blindly introduce patches there, which
break this design, you can return to your init revision fairly soon.

Also, when a patch introduces some function, there may be uses for this
function which the author didn't think about. Others may suggest other possible
uses and options of the function, author can extend it, other can modify other
parts of code to use it etc.  Obviously this can be done in CVS, but when
anyone won't know that function was ever introduced, it is hard to suggest any
improvements before someone browsing the code will randomly notice it and start
wonder what it is for.

Also, the code still must look at least partially reasonable.  Generally, it
must be somewhat commented, IMHO. This is thing which indent won't do for you,
and other will spend unneccessarily much time upon that, when you know far best
what it does so you can figure out some comments fairly fast.

> And one other thing: you can't always get there from here via small patches.
> Some things require large patches and there's no other way around it.
Example?

>             Furthermore, a set of small patches isn't necessarily
> going to be higher quality as a whole than a single large patch.
> Oftentimes, quite the opposite will be true: the large patch will have
> more cohesion because it's written by a single person with a very
> clear idea of what's going on.
I wasn't talking about set of small patches by various persons. I was talking
about set of small patches splitted up by their author in order to be easily
reviewable.

> > >                   A patch should be disapproved only if it breaks 
> > > something
> > > in a very fundamental way.
> > This way you can't easily check if it breaks something or not.
> 
> Sure you can: let the playtesters at it.  That's the whole *point*
> behind accepting the patch for inclusion in the main (or, I prefer,
> development) tree.
See above.

-- 

                                Petr "Pasky" Baudis

UN*X programmer, UN*X administrator, hobbies = IPv6, IRC, FreeCiv hacking
.
  "A common mistake that people make, when trying to design
   something completely foolproof is to underestimate the
   ingenuity of complete fools."
     -- Douglas Adams in Mostly Harmless
.
Public PGP key, geekcode and stuff: http://pasky.ji.cz/~pasky/