Code review comment for lp:~kicad-gost-committers/kicad/kicad

Revision history for this message
jean-pierre charras (jp-charras) wrote :

The intermediate file is not an intermediate file:
it is *the* new netlist format, which exists under 2 format:
a S-expression format (easy to parse with the s-expr internal parser)
a xml format: the same file, easy to parse with external xml parsers.
This netlist contains all data to create BOM and boards ...
Many post processings uses a netlist, so I do not understand why is NOT the right approach.

And yes, using this approach is fully transparent to the user, once the comand line is initialized (see the netlist dialog). Only one click to generate the file. And the user does not know there is an intermediate file (kicad already uses intermediate files).

Yours cons are irrelevant:
1 - the guy has not even had a look to files in kicad (kicad/python)
2 - there is no setting (just a command line which could be predefined)
3 - intermediate file does not change as fast as the kicad internal code.

For kicad2pcad, you forgot what I said you:
1 - Pcbnew includes a plugin mechanism to import files, use it.
2 - the file to convert was a S-expr files, and pcbnew has a very good S-expr parser. Why to write your S-expr code parser to create an inttermediate file with a new parser, this was duplicate and/or useless code.

This is very different from the BOM generation

About Russian literals, one time more, I am saying I am talking about internal Kicad code.
Your patch come with not comments, and many sentences not readable for non Russian contributors
It means this work cannot be modified and used by some other contributor which want mo modify the BOM to make their own BOM format.
Each company has its BOM format, therefore GOST format is only one of multiple existing formats.
Creating code which cannot be understood by contributors is not the spirit of free open source software.

Besides, the patch creates a heavy dependance of Kicad sources with openoffice/libreoffice sdk.
This is a *major drawback*.
This can be easily avoided by a fully external script.

If it is only for Russian users, no problem with Russian literals.
But if it is only for Russian users, there is no room for this patch inside Kicad code.
Code inside Kicad is made for everybody.

Therefore I cannot accept your patch.

« Back to merge proposal