← Back to team overview

kicad-developers team mailing list archive

Re: 3D-Viewer - Request for merge evaluation

 

I haven't looked at all of your latest commits but I saw quite a few
coding policy issues in your code.  Source file maximum line length is
100 characters with an exception for long strings (although I prefer
that they be broken into smaller strings).  I saw a lot of line wrapping
in your code.  There are quite a few places where you missed spaces
between function call parameters and parenthesis.  You have some nested
for() loops where the inner loop is not indented.  While not required, I
would prefer you put the inner for() loop inside curly brackets as well
for improved readability.  Doxygen comments belong in header files
unless it's local to the source file.  You have inlined if statements.
Please put the statement after the if() on the next line and indented.
Move your comments to the line above the code rather than going past the
100 character limit with inline comment.

I'm fine with committing this once the coding policy violations have
been resolved.  Anyone else have any objections to this being merged
into the product branch?  Speak now or forever hold your peace.

@Mario, please keep you branch synched up with the product branch so
I'll be sure to get a clean merge.

Thanks,

Wayne

On 6/28/2016 2:52 PM, Mário Luzeiro wrote:
> Hi Wayne,
> 
> From the email below, I took your advice about the camel case and underscores naming and updated my branch,
> renaming variables and some function names, to apply to that policy.
> 
> https://code.launchpad.net/~mrluzeiro/kicad/kicad_new3d-viewer
> 
> I didn't received any more reports related with freezes or crashes.
> I am not planning at moment any addition, I will just keep it updated with main branch.
> 
> Regards,
> Mario Luzeiro
> ________________________________________
> From: Kicad-developers [kicad-developers-bounces+mrluzeiro=ua.pt@xxxxxxxxxxxxxxxxxxx] on behalf of Wayne Stambaugh [stambaughw@xxxxxxxxx]
> Sent: 27 June 2016 19:40
> To: kicad-developers@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Kicad-developers] [PATCH 03/16] Clarify ERC: we're iterating netlist items, not nets
> 
> This patch and patches 0005 and 0006 of this patch set do not apply
> cleanly.  Please rebase and resubmit them when you get a chance.  FYI,
> the coding policy frowns upon mixing camel case and underscores.  I know
> there are cases of this in the code but they are most likely left over
> from before the coding policy was implemented.  Please rename
> lastItem_idx to lastItemIdx or last_item_idx.  The same goes for
> nextItem_idx.
> 
> Thanks,
> 
> Wayne
> 


Follow ups

References