← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Ortographic mode for 3D viewer and minor cleanup

 

On Oct 7, 2010, at 15:03 PM, Dick Hollenbeck wrote:

> On 10/07/2010 07:50 AM, Alex G wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>> 
>> On 10/07/2010 08:53 AM, Dick Hollenbeck wrote:
>> 
>>> Alex,
>>> 
>>> congrats on your patch getting accepted and applied!
>>> 
>>> I have just a few comments below that will make your next patch even better.
>>> 
>>> Thanks,
>>> 
>>> Dick
>>> 
>>> 
>> Dick,
>> 
>> Thanks for the confidence. :)
>> I'm looking forward to making more contributions in the future,
>> hopefully, of more value. Although my narrow experience makes tasks such
>> as working on the new library structure seem daunting, I'm constantly
>> looking for something I can improve.
>> 
> 
> We'd like to see you contribute more.
> 
> On the enums' commas, I usually leave them in there with the notion that
> you will be adding more and, why waist the time to go back and add that
> comma to extend the list?
> 
> The language designers thought the same thing after a point.  This was
> not originally in the language and they it added after somebody asked
> for it.
> 
> So now comes the compiler developer, who probably does not have as much
> experience as the language designer, and ties that warning to a
> -pedantic flag.  That is fine, since he also gave you the ability to
> turn off warnings on a case by case basis, which what I would suggest
> you do.  (Whatever way you are controlling the compiler flags.  I just
> let the cmake script do that for me.)
> 
> It is part of the language, and many experienced programmers utilize it,
> and any compiler supporting C++ should handle it without voicing an
> opinion on it if asked to be quiet about it.
> 
> And I cannot believe I actually spent all the time to write this.
> 
> But getting you to contribute more is worth it.  Keep up the good work!
> 
> Dick

If you want to silent it use:
-std=c99 -pedantic

without the std option gcc uses the "more" pedantic c89 specs

/Martijn

> 
> 
> 
>> I would like to explain why I made some modifications that are more or
>> less debatable. I am not trying trying to prove that I am right, just to
>> share some thoughts.
>> 
>> I compile with -pedantic. I made this habit while working on a
>> cross-platform HPC project. -pedantic made sure that the code would work
>> with other compilers (including MSVC++), but also caught a lot of small
>> mistakes that could cause the program to segfault. I find it a very
>> useful tool. Although a very anal one, I find its benefits greater than
>> the sum of its deficiencies.
>> 
>> When I first built kicad with -pedantic, the number of warnings I got
>> made it impossible to catch the meaningful ones. The extra "," at end of
>> enumerator list was one of the more common ones. While the lack of ","
>> is noise for you, its presence is noise for me. I believed that removing
>> it would not cause any stir.
>> 
>> 
>>> Looks like you had one to many spaces on these lines, should have been 4.
>>> 
>>> 
>> Sorry about that. Careless mistake on my part.
>> 
>> 
>>> Spelling error in ortographic
>>> 
>>> 
>> Careless mistake again.
>> 
>> 
>>> Please put a blank line above comments if comments are only thing on
>>> that line and they are not a comment continuation.
>>> 
>>> 
>>> needed a blank line here, above the single line comment.
>>> 
>>> 
>> Of course.
>> 
>> 
>>> No need for this edit, last enum can have a trailing comma just fine.  I
>>> write code like that.
>>> 
>>> ditto, leave the comma in, this edit is noise.
>>> 
>>> More noise, and more noise many times below that I do not mention again.
>>> 
>>> Noise.
>>> 
>>> 
>> - -pedantic made me do it. I'm innocent :P
>> 
>> 
>>> The space between \n E is not needed, removing the \ before the E was a
>>> good edit.
>>> 
>> - -pedantic pointed me to this :)
>> 
>> 
>>> selection spelling
>>> 
>>> 
>>>> +    int           m_Selected_Tool;              // Pour editions: Tool (Dcode) selectionn�
>>>> 
>>>> 
>> Didn't catch that. I had some files that KDevelop was complaining about
>> the encoding, and this was one of them. I failed to see why the
>> complaint occurred, and as a result, failed to see the horrible monster
>> I had created. I apologize about this.
>> 
>> 
>>> comment start is off by one:
>>> 
>>> 
>>>> +    FILLED_WITH_BG_BODYCOLOR    /* Poly, Square, Circle, Arc = option Fill
>>>>                                   * with background body color, translucent
>>>>                                   * (texts inside this shape can be seen)
>>>>                                   * not filled in B&W mode when plotting or
>>>> 
>>>> 
>> Careless mistake.
>> 
>> 
>>> These were my printf()s and I can assure you that nobody else needs them
>>> except me.  I like printf() as a better IO model than std::out <<
>>> 
>>> 
>> Some pedantic warnings as well led me here. I thought that std::cout
>> might be better because of stronger type-safety. If you use %p, can you
>> please cast the pointer to (void *) ? It fixes the -pedantic warning.
>> 
>> 
>>> So please don't do this in the future.  
>>> 
>> I don't understand why printf() is preferred over std::cout in C++ code,
>> but I will comply.
>> 
>> Alex
>> -----BEGIN PGP SIGNATURE-----
>> Version: GnuPG v1.4.10 (GNU/Linux)
>> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>> 
>> iQIcBAEBAgAGBQJMrcH3AAoJEL0kPHNPXJJKsX0QAIc0A1fqwGk37nQj0EhDgICu
>> s4rb0EzQXHueWcwii7sqVsksNGQuWC6aqhBXeFBVxeT9hp2bRJsp/+K4OwLwLWw1
>> p27Q+shJxUlVe44Ra8Lz/BkvePoJlRDtHyVH8DLd26kuyry+ApYvMnXuUZ7KUzhV
>> nWM6alyWHEorm6xF5JUjQz7zXtVsDMye2ML83YkETej+wQ9mQrdhgk7JcHnrdnLe
>> avx+9KRrTOGGY1O/wfmIvE+8ewYPBzEGo++obQo25xbAGge848M/dm3nz1sGQEFa
>> SY7mYnoXFXw7j6dkEQxUCFapNwhqxEAWJQlUecRYLoHIE7ZVN0dLh5j+is0fSbvP
>> 4EX78pFk37PI/bNEA9ReYENQV5IXJoWRp/d6xbCpUh1bIxPD8kVlwt45Bv9V9D6w
>> btLp3LrIBNt8fOK3M+6h5t20gDDslPcNsP9VrTo0CSt2N8TuXBJVmlD8wnpQ56+9
>> EeDfKDmInP+xt4Ca6M5S3mENYoZ7Hj8wrCxX+xQ+asZ6PGCpD0PpcCfdWazFCx2I
>> akdXNnpgisoS4jQXygkPl8uXmEm5NvP7aHpR05MMRiftMFHnzlr21rpihOXawFr2
>> Cm6j3RPcOT8tvEwz9fpsbce2kDK3g9hdkCSOqzncmctaWC8iUh5dGOTWuQCD1uWi
>> YO5+kasFEpaGhTjYeB9H
>> =jpVM
>> -----END PGP SIGNATURE-----
>> 
>> _______________________________________________
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
>> 
>> 
> 
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp




References