← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Make SVG import a CMake option

 

Le 04/12/2018 à 16:14, John Beard a écrit :
> Hi,
> 
> This patch makes SVG import a CMake option. This allows interested
> developers to work on this feature without having to maintain a patch
> on top to disable the define. While possible with Git, it is certainly
> a bit fiddly.
> 
> The CMake option is KICAD_EXPERIMENTAL_IMPORT_SVG. Set to ON to use SVG import.
> 
> I have documented this in the Pcbnew CMake, rather than the top level
> CMake, as it's a developer option, and should not be set by general
> packagers. This keeps it near the set_property call that sets the
> define in the file needed (which will be removed, along with the
> compile flag when SVG import is done).
> 
> Now for a proposal:
> 
> I think ideally, this would not even be a #define, but instead a run
> time option, to eliminate any dead code in the master branch that is
> not truly required. Dead code is vulnerable to bit rot. I think we
> should have a way to define and set experimental and advanced options
> for fine configuration. Somewhat like Firefox's about:config options,
> but without the UI (unless someone wants to make that effort, of
> course!)
> 
> Shipping development features in the main branch, as built code, but
> protected by run-time feature flags is a common practice, as it not
> only makes it easier for developers to work on the code, but it gets
> the code, as-is, into the CI, analysis and testing systems ASAP, and
> bugs can be found sooner rather than during a big code dump from a
> private branch, when it's easy to miss one bug out of many.
> 
> This will allow to use custom configs for control of things that
> currently need recompilation to achieve. It will also hopefully
> encourage people to write parametrisable code, which is likely to be
> more testable code. I suggest perhaps placing them in a separate
> "advanced" config file, so that you can use XDG_CONFIG_DIR to switch
> between multiple configs if wanted during testing. This is VERY
> helpful for A/B testing.
> 
> The implication of these advanced configs is that if you set them for
> development, or for or advanced use only, and non-default settings are
> not officially supported. If a setting is deemed useful enough to
> become supported, it should be given UI and put into the existing
> config files. Perhaps any set advanced config should be included in
> the version info dump.ki
> 
> Cheers,
> 
> John
> 

Hi John,

I committed your fix. Thanks.

-- 
Jean-Pierre CHARRAS


References