kicad-developers team mailing list archive

[Dick Hollenbeck <dick@xxxxxxxxxxx>]

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



> 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
>
>