[Freeciv-Dev] Re: [UPDATE] Corecleanup_08 patch to cvs-Sep28

["Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>]

I understand your hope that things might be easier to manage in smaller
chunks, but I think that you are fooling yourself. The review is 
probably actually easier working with a clean consistent version than
something that is a bastard amalgamation of half-hearted and partial
changes. 

It is also easier to just look at an updated snapshot vs a current
CVS in side-by-side views for those things you are interested in, as 
opposed to reading diffs.

Diffs in themselves are not that readable, and you will have a lot of
difficulty using/applying random partial diffs without the supporting 
changes. Or I will have a lot of effort redoing changes to fit with a 
broken CVS and all possible orders that you might decide to apply them 
if the splits are into Raimar's 500 line limits - this is just nonsense
make work designed to obstruct and not advance Freeciv development, and
there are a lot of similar current practices :-).

It is not productive to have to interpret, deal with or test each 
artifical division of a consistent set of changes, with all the extra 
hacks needed to make it work with broken code that is being fixed by 
a whole series. Certainly not for review and basic understanding which 
is where the process is usually blocked.

At least with the tested working snapshot, you can play and experiment
in a consistent environment. 

Whereas, in your case, if you try to play with just the AI code applied
to a current CVS, then you are neither looking at it in the environment
it was built and tested in, nor are you likely reviewing the changes
but just rediscovering already found bugs and problems :-).

The mess of partial changes that is current CVS attests to the viability
of doing a large standardization effort in random bits and pieces! 

And frankly, I really *am* tired of doing yet-another-split everytime 
someone wants to make a spurious cosmetic change BEFORE they actually 
invest any of their own time reviewing what should be the critical 
code and algorithmic changes. It takes 5 minutes to unpack build and 
start playing with the current snapshot, if you follow the instructions.

It is maybe time to *try* a slightly different approach :-).

This is a development snapshot of the parallel development tree as per 
Raimar's request after the last update :-).

When it is time to actually code review CVS patch candidates we can talk
about what is to go in, and do the last cycles on a consistent set of
bite-sized pieces. For now this is code you can read and experiment with 
to gain understanding and knowledge of the system and that should suffice.

If you want to extract pieces of it for whatever purposes, it is out
there in the public domain for whatever benefit it provides. If you can
look at what is here and come up with better, I'm all for it. 

Meantime, I'm having fun playing with these new features ... :-)

Cheers,
RossW
=====

FYI: 

1/3 of the patch is mapgen replacement, and it would drop by 80%
if one just sent the replacement file as opposed to the diff :-).

1/3 (or maybe more) is reformatting changes from introducing iterate
macros or other code updates that change (or fix) indentation. I
maintain, the code is more understandable after reformatting especially 
in the AI code :-).

You don't need to worry about any of the client/GUI code changes that
basically put things back to the way they were before the recent 
amalgamation of the GUI and core direction system broke them, or almost 
back. 3-4 routines in each of the ai and server directory acount for most 
of the AI stuff, with maybe a smattering of common utilities.

What you want removed is probably less than 10% of the total. And it
is easily recognized as dealing with map addressing fixes. Most of the
AI changes aren't map addressing related, so there is not much overlap
to confuse you anyway.

And to put it in perspective, there have been 2-3 "patches" applied to
CVS recently that were at least 50% of this size, or at least as
intrusive in the numbers of files they actually touched.

At 03:55 PM 01/10/03 +0100, Gregory Berkolaiko wrote:
> --- "Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx> wrote: 
>> This patch updates the corecleanup sets of changes to Sep 28, 2001.
>> 
>> In additional to the suggested fixes to corecleanup_07 and ongoing
>> updates to the current CVS, there is an extensive cleanup of the core
>> server AI code. It should now be more readable, correct and has been
>> updated to change the AI strategies in ways that move them from the 
>> existing xenophobic military despotism to those compatible with
>> higher forms of government.
>
>I know you are tired of these requests to split your patches, but I would
>really like to have a look at your changes to ai without having to worry
>about the underlying coordinate system.  So could you extract the changes
>to ai from you patch please?  It shouldn't be too hard for somebody who
>knows the patch but for me it was impossible unless I really want to go
>into the question of local city coordinates.
>
>G.
>____________________________________________________________
>Do You Yahoo!?
>Get your free @yahoo.co.uk address at http://mail.yahoo.co.uk
>or your free @yahoo.ie address at http://mail.yahoo.ie