[Freeciv-Dev] Re: [PATCH] Formatting cleanup.

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Fri, Sep 14, 2001 at 05:18:01AM +0200, Gaute B Strokkenes wrote:
> On Wed, 12 Sep 2001, hawk@xxxxxxxxxxxxxxxxxxxxxxx wrote:
> > On Tue, Sep 11, 2001 at 10:07:45PM +0200, Gaute B Strokkenes wrote:
> >> 
> >> This patch contains the formatting bits of the patch that I posted
> >> earlier containing various map coordinate related cleanups.  The
> >> reason I'm posting this separately is that my changing for loops to
> >> use iter macros is causing some very obscure and hard to track down
> >> changes in server behaviour (as exhibited by running a long AI game
> >> and comparing the save game files), so I would like to apply this
> >> ASAP to avoid having to drag this stuff around forever in my local
> >> tree.
> >> 
> >> Apart from that, there is a new function, nearest_real_pos().
> > 
> > IMHO you should have removed the formatting changes you
> > applied. Either don't do them or go the whole path (global indent
> > run).
> 
> That won't work.  There is no indent program that will correct typos
> in comments, rename oddly-named variables or remove redundant
> parantheses.  Good style is not something you can impose simply by
> waving a magic wand, though I will admit that running Freeciv through
> indent all at once would probably result in something nicer than what
> we have now, even if it's not perfect.
> 
> Doing what I did to part of the source to all of the source would take
> an ungodly amount of time and would be error prone because of the
> repetitiveness of the task.  Bit-by-bit is the only way to fly here.

Cleanup in general is good and yes for some tasks there are no
tools. My argument is that if you do such changes (add space after
variables declaration, remove () from return,...) do:
 - one item per patch
 - go over the whole code and correct all code
 - make an entry in freeciv_hackers_guide.txt

You attempts are half-hearted and are will go under in the noise of
the future development.

If you think that it to much work to go through the whole code: don't
do it or write a script.

> The only argument I could think of against this sort of thing is that
> somebody might be touching the same code and would then be forced to
> play the game of CVS conflict resolution.  Though.  In the long run,
> having a consistent coding style helps keep that sort of thing at
> bay.

> There is also the "careful-it-might-break" argument.  Given the
> extensive testing I put this patch through I am pretty sure I didn't
> break anything.

I don't see much risks here for such syntactical changes.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Premature optimization is the root of all evil."
    -- D. E. Knuth in "Structured Programming with go to Statements"