kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

Hi Brian,

On 3/12/2019 3:37 PM, Brian wrote:
> 
> 
> On 3/12/19 3:30 PM, Wayne Stambaugh wrote:
>>
>> I still have to finish reviewing the original patch so please do not
>> merge it.  For the few simple changes required to access the symbol
>> library parser and formatter that Brian requested, it seems like an
>> excessively large change set.
> 
> Oh yes, the patch I submitted contains both the serialization and an
> implementation of a SCH_PLUGIN registry.  Given the discussion so far, I
> would expect you to reject it as a whole; I wouldn't expect you to piece
> through it and pull out only the serialization changes.  In a day or two
> I should be able to submit a patch just for the
> LEGACY_PART_READER/WRITER classes, which should only touch three files. 
> I just need to figure out the right git gymnastics, since I didn't
> commit the work to my branch in separate pieces.
> 
> Cheers,
> -Brian
> 

I wouldn't have pieced through it but I still want to review the code
before you do any more work just so the changes make sense to me.  I
wrote most of the original board plugin and schematic plugin code so I
know what I have in mind moving forward so I need to make sure your
changes fit with that plan.

As for patches, it's always a good idea to make commits the smallest
required size to implement your changes.  Large change sets like the one
you posted are difficult to review and comment on.  I did notice a quite
a few coding policy[1] errors.  Mostly trailing white space and some K&R
curly bracket indentation issues.  If you don't want to be bothered with
learning the KiCad coding style, there is a git commit hook (see section
1.3 in the coding policy) that can be called at commit time that will
format your code correctly.  You need to have clang-format installed for
the commit hook to work.

Cheers,

Wayne

[1]:
http://docs.kicad-pcb.org/doxygen/md_Documentation_development_coding-style-policy.html