Code review comment for lp:~cern-kicad/kicad/bugfix_1275319

Revision history for this message
Dick Hollenbeck (dickelbeck) wrote :

On 02/06/2014 11:53 AM, Dick Hollenbeck wrote:
> On 02/06/2014 08:17 AM, Maciej Sumiński wrote:
>> Maciej Sumiński has proposed merging lp:~cern-kicad/kicad/bugfix_1275319 into lp:kicad.
>>
>> Requested reviews:
>> kicad-product-committers (kicad-product-committers)
>> Related bugs:
>> Bug #1275319 in KiCad: "pcbnew OpenGL PNS router does not show track while routing"
>> https://bugs.launchpad.net/kicad/+bug/1275319
>>
>> For more details, see:
>> https://code.launchpad.net/~cern-kicad/kicad/bugfix_1275319/+merge/205166
>>
>
>
> on a 32 bit platform, a long and an int are both 32 bits.
>
> I stopped reading the patch after I saw that. I don't like long where an "int" will
> suffice, or where an int and a long are the same thing. long really should never be used.
> Because if you need a certain bitness, then there are datatypes which ensure that.
>
> int64_t comes to mind.
>
>
> The rest seems like mostly noise and renaming. Where is the actual fix? What lines,
> maybe you can name two or 3 that fix the code.
>
> Dick
>

I do not mind the renaming and the clean up, just the long.

(But amidst the renaming, was unable to see the fix.)

Go for clean up where ever you can! Just recognize what that means :)

« Back to merge proposal