kicad-developers team mailing list archive

[Seth Hillbrand <seth@xxxxxxxxxxxxx>]

On 2019-10-14 14:42, Zficani Zficani wrote:

> Hi,
> No problem, I just wanted to make sure I sent the message properly. 
> Here's a single squashed patch with all previous changes and these 
> comments about copying selection.
>
> Thank you so much for your review.

Hi Zficani-

The functionality feels correct and I really like it.  Here are a few 
comments on the current patch:

1) I would prefer that the disabled options in the Annotation page are 
grey (disabled) and not hidden when the option is unchecked.  This 
reserves the correct space for them when we add options in the future.

2) Please double-check your code formatting.  Spaces inside the 
parentheses are missing in a few spots.

3) Don't use C-style casts.  C++ static_cast<type>() is preferred.

4) Single-line statements after if/else don't get brackets {}

5) I think that pasting Unit B of a component should paste as the first 
missing Unit B in the schematic and not the next open annotation number. 
 See the attached image for the result of duplicating a quad op-amp for 
an example of this problem.

This will be a great addition to KiCad.  Thank you for taking this one 
on!

Best-
Seth

Seth Hillbrand
KiCad Services Corporation
+1 530 302 5483 | +1 212 603 9372
www.kipro-pcb.com
Davis, CA

Attachment: DuplicateUnits.png
Description: PNG image