Durk wrote

> Sent: 13 June 2007 19:25
> To: FlightGear developers discussions
> Subject: Re: [Flightgear-devel] C++ code beautifier / Coding 
> standardsproposal
> 
> 
> On Saturday 09 June 2007 17:07, Melchior FRANZ wrote:
> > * Durk Talsma -- Saturday 09 June 2007:
> > > I've attached the configuration file, I've used for a few tests. 
> > > Would it be an idea to base a coding standard on the 
> basis of this?
> >
> > I would have been interested in this dicussion, but how can 
> you dare 
> > asking and *at the same time* starting to commit 
> auto-formatted files 
> > in a not agreed-upon style that is completely different from the 
> > original style? Fait accomplis?
> >
> > m.
> >
> 
> Since not everybody in the development team may immediately 
> realize what 
> Melchior is referring to, let me add a little backstory. The 
> start of this 
> thread was motivated by indentation inconsistencies in a file 
> that I was 
> patching last Saturday. This file, AIModels/AIAircraft.cxx  
> is one that I 
> have been putting a considerable amount of work in in the 
> past, and know 
> quite well.
> 
> In the past, we have had discussions on the use of 
> autoformatting tools, but 
> the general consensus was that most tools, in particular 
> "indent", did not 
> handle C++ (as opposed to plain old 'C') very well. However, 
> that discussion 
> was years ago, and I found this to be a good opportunity to 
> start looking for 
> a decent indentation tool again. 
> 
> I came across this little program bcpp, which I referred to 
> in the beginning 
> of the thread. After some tests, I sent out the email that 
> started this 
> thread. Then I ran bcpp on a copy of AIAircraft.cxx, and was 
> -in general- 
> very pleased with the new consistency. I double-checked the 
> whole file by 
> hand to see if there were any apparent inconsistencies, and 
> when I determined 
> the reformatted file was in a much better shape than the 
> original one, I 
> decided to go ahead an commit the modified version. After 
> all, some people 
> are rather pedantic about code formatting, so I figured these 
> very people 
> would be pleased by the new -tightly formatted- layout of the file.
> 
> Unfortunately, accidents do happen, and it seems like this 
> were the case here 
> as well. Personally, I don't really have a strong preference 
> for a particular 
> indentation style, so I committed the file the way it was. 
> Unfortunately, it 
> seems that this heated-up Melchior to the point spontaneous 
> combustion. The 
> upside is that version numbers are cheap, and revision 
> numbers are even 
> cheaper. So, in the wake of this all, I have committed 
> another revision in 
> strict K&R/4 style, hoping that everybody would be happy 
> again. Incidentally, 
> the bulk of that reformatting was done using the astyle 
> (artistic style) 
> program that Norman Vine referred to. 
> 
> Let me emphasize that it is *not* my intention to reformat 
> the entire code 
> base. AIAircraft.cxx was functionally restructured anyway, as 
> a consequence 
> of the patch I was committing, so this was an ideal time to 
> fix-up layout 
> issues. Since this would require a lot of work anyway, I 
> decided to conduct a 
> limited scale experiment by evaluating some beautifier tools. 
> Since the 
> results were favorable in my evaluation, I decided to push 
> ahead, and not let 
> the results of the experiment get wasted.
> 
> In light of the this, I would like to reopen the discussion 
> again. In the 
> past, it was decided not to use beautifiers, because of their 
> inability to 
> handle C++. But it seems like those days are gone. The tools 
> that I did check 
> out seem to be doing a good job on C++ code, and and can 
> configured quite 
> easily to do formatting into an acceptable style. For 
> FlightGear, it seems 
> like this would be Kernigan and Richie, using 4 spaces. 
> (K&R/4) for each 
> level of indentation. I still believe it's too early, to go 
> with a general 
> recommendation in favor of using a beautifier. However, I 
> don't really see 
> anything wrong with occasional use of one of these tools done 
> by one of the 
> regular committers, in particular when it's done in 
> combination with a 
> careful evaluation of the results and when circumstances favor such a 
> reformatting. 
> 
> Cheers,
> Durk
> 

This war over formatting has obscured what actually happened. We got a
totally unexpected patch which apparently hadn't been tested, or reviewed by
anyone who had the least idea of how AIModels worked or was meant to do. It:

A. Didn't compile under MSVC8.

B. Broke Air-to-Air TACAN. (thanks Alexis for a neat fix)

C. Broke AAR

D. Has probably (but I haven't checked yet) broken MP AAR

It might be the most elegant style, and good programming, but has rendered
the tanker facility totally inoperative. Not Well Done, or in NATO terms
negat Bravo Zulu.

Lesson for the future. Understand what code does, test that it doesn't break
anything, make sure it compiles on at least one other os, and submit it for
review (especially when you are mucking around with other people's code).
Then it just might be accepted.

I don't enjoy repairing code which other people break, and nor, I expect
does Alexis. I haven't got time, nor the inclination. Given that we are
trying to get a release out, I would strongly recommend that this patch be
reverted until it works. I for one do not wish go forward with a broken AAR
facility.

Not best pleased 

Vivian

  

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Flightgear-devel mailing list
Flightgear-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/flightgear-devel

Reply via email to