[Freeciv-Dev] Re: PATCH: AI cleanup Version 2

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Sat, Nov 24, 2001 at 03:20:44PM +0100, Petr Baudis wrote:
> > My job is it to verify such patches. It would make my job much easier if you
> > split such patches. For example in "move comments around", "rename
> > variables", "move variables around" and "add comments". The patch below it 
> > is
> > still ok but I just want to show my position.
> I also maintain some projects so i get it :-). However, usually maintainers
> want to split into tokens patches coping with different areas of code, or
> adding different features. I'm willing to give you extra patch for each [set
> of] function[s], but splitting it to renaming variables and moving comments 
> etc
> will be hard. I do all the tasks at once, as I first have to reformat the code
> in order to be able to read it reasonably. As I'm reformatting the code, I 
> also
> move comments, and as I read the comments, I also start to understand some of
> the variables and rename them while reformating.  And as I start to understand
> some variables, I start to understand the code itself and also write some
> comments. It would be very difficult for me to do this tasks one-by-one, and
> also very time consuming. I hope you understand my position too.

It is ok if you split the patch in other way.

> > Also note that the patch contains extra reformatting noise.
> I said, definitively not a noise. AI code is not ugly only because there are
> ugly and confusing variable names, confusing and strange and missing comments,
> but also because the code formatting itself is a lot ugly, and that counts a
> lot as well. At your advice, I gave up making really unneccessary indendation
> changes, I edited the patch manually to remove all space-to-tabs-only lines,
> and afaik only thing which rest was addition of some blank lines. Without 
> them,
> it is a lot confusing, at this has to be done sometimes, so let's do it now,
> when we are cleaning up the code and when we still understand which logical
> blocks keep together.

If we do so we have to have:
 - to do it for all code
 - guidelines first

Doing it at some part of the code based on your personal idea of how
does it look nice is not ok. We all have personal idea of how does the
code have to look but have to formalize this before we go to change
this. I really hope we can get a discussion going which will produce
such guidelines. Here are some questions for the start:

   * init vars
       int foo(struct city *punit)
       {
         int x=punit->x;
       } 
or
       int foo(struct city *punit)
       {
         int x;

         x=punit->x;
       }
   * empty line after vars
       int x;
       x=3; 
or
       int x;

       x=3;
   * comments
       x=3; /* assign 3 to x */ 
or
       /* assign 3 to x */
       x=3;
   * extra {} on iterates

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "The BeOS takes the best features from the major operating systems. 
  It's got the power and flexibility of Unix, the interface and ease 
  of use of the MacOS, and Minesweeper from Windows."