kicad-developers team mailing list archive

[Alex G <mr.nuke.me@xxxxxxxxx>]

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

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