← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] common.h tidyups

 

Hey John,

I'll cherry-pick the swig commit from your branch.

The revised patch you sent is now failing to build
eeschema/sim/ngspice.cpp.  It's missing the definitions for wxTextFile
and wxFileName so you will have to add the appropriate headers to
ngspice.cpp.

Cheers,

Wayne

On 3/24/2017 11:38 AM, John Beard wrote:
> Hi Wayne,
> 
> Like this?
> 
> As for the patches that don't apply out of order, in this case it is
> due to removing the following line in the previous commit. If you want
> to cherry-pick that patch, doing it by git rather than raw patches
> would be much easier. Git would be able to flag the conflict up and it
> should be a simple one to resolve.
> 
> In this case, the conflict caused by cherry-picking "Remove reference
> to InitKiCadAbout in kicad.i SWIG file" without the preceding commit
> would be:
> 
> <<<<<<< HEAD
> %ignore InitKiCadAbout;
> %ignore GetCommandOptions;
> 
> =======
>>>>>>>> adce35d99... Remove reference to InitKiCadAbout in kicad.i SWIG file
> 
> After resolution, it would be just:
> 
> %ignore GetCommandOptions;
> 
> Cheers,
> 
> John
> 
> On Fri, Mar 24, 2017 at 11:04 PM, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>> On 3/24/2017 9:50 AM, John Beard wrote:
>>> Hi Wayne,
>>>
>>> The InitKicadAbout function was removed in 2010 by commit
>>> b45a35b719739e1ed1a723a4e7412d63020e5f2c. Is it still useful in the
>>> swig file?
>>
>> It should be removed but unfortunately this patch now no longer applies
>> cleanly.
>>
>>>
>>> As for the directory_utils compile failure, perhaps there's a
>>> difference in WX on Windows?
>>> Does #include <wx/config.h> at the top of common/directory_utils.cpp fix it?
>>
>> You need to add #include <wx/fileconf.h>.  On windows <wx/confbase.h>
>> defaults to the wxRegConfig and wxFileConfig is not exposed.
>>
>>>
>>> Cheers,
>>>
>>> John
>>>
>>>
>>>
>>> On Fri, Mar 24, 2017 at 2:21 AM, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>>>> John,
>>>>
>>>> I committed up to patch 9.  Patch 6 did not apply cleanly.  Apparently
>>>> it needs the broken patch 5 so it will need to be rebased once you fix
>>>> patch 5.
>>>>
>>>> I have a question about patch 10.  Did you test InitAboutKicad from the
>>>> python scripting.  It was originally ignored because there were issues
>>>> so I just want to be sure before I commit this patch.
>>>>
>>>> Thanks,
>>>>
>>>> Wayne
>>>>
>>>> On 3/23/2017 9:25 AM, Wayne Stambaugh wrote:
>>>>> So far, I've tested patches 1-4 and they seem fine.  Patch 5 fails to
>>>>> compile on windows with the following compiler output:
>>>>>
>>>>> C:/msys64/home/wstambaugh/src/kicad-trunk/common/directory_utils.cpp: In
>>>>> function 'wxConfigBase* GetNewConfig(const wxString&)':
>>>>> C:/msys64/home/wstambaugh/src/kicad-trunk/common/directory_utils.cpp:78:15:
>>>>> error: expected type-specifier before 'wxFileConfig'
>>>>>      cfg = new wxFileConfig( wxT( "" ), wxT( "" ),
>>>>> configname.GetFullPath() );
>>>>>
>>>>> I'll commit what I've tested thus far and continue testing the remaining
>>>>> patches as time permits.
>>>>>
>>>>> On 3/23/2017 9:17 AM, Maciej Sumiński wrote:
>>>>>> Hi John,
>>>>>>
>>>>>> Thank you for the patches. I have committed:
>>>>>>   0001 OPENGL_GAL: Init currentTarget
>>>>>>   0003 Rework zone creation in GAL (committed yesterday)
>>>>>>
>>>>>> I had a look at other changes, and they appear to be a sensible common.h
>>>>>> clean-up. All patches received the Orson Approves™ badge, but I leave
>>>>>> the final decision to Wayne.
>>>>>>
>>>>>> Regards,
>>>>>> Orson
>>>>>>
>>>>>> On 03/22/2017 03:12 PM, John Beard wrote:
>>>>>>> Hi Wayne,
>>>>>>>
>>>>>>> They are pretty much all independent in scope, but there might be
>>>>>>> (very minor) conflicts if you apply a subset or out of order.
>>>>>>>
>>>>>>> Here are the patches.
>>>>>>>
>>>>>>> Cheers,
>>>>>>>
>>>>>>> John
>>>>>>>
>>>>>>> On Wed, Mar 22, 2017 at 10:03 PM, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>>>>>>>> Hey John,
>>>>>>>>
>>>>>>>> I'm assuming that the common.h tidy ups are from commit
>>>>>>>> cb2ef9ea5e4ee449295c41573beefa2fc0935b84 forward.  If you don't mind, it
>>>>>>>> would make my life a bit easier if you would send the patches.  They
>>>>>>>> look like they are all stand alone changes so it shouldn't be an issue
>>>>>>>> to push them one at a time.  If someone else can get to it before me,
>>>>>>>> I'm fine with merging these changes.
>>>>>>>>
>>>>>>>> Cheers,
>>>>>>>>
>>>>>>>> Wayne
>>>>>>>>
>>>>>>>> On 3/22/2017 6:34 AM, John Beard wrote:
>>>>>>>>> Hi Wayne,
>>>>>>>>>
>>>>>>>>> Just an update: the tidy_ups branch [1] has been rebased and a small
>>>>>>>>> number of conflicts resolved (mostly includes being added in the same
>>>>>>>>> places).
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> John
>>>>>>>>>
>>>>>>>>> [1] https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/tidy_ups
>>>>>>>>>
>>>>>>>>> On Thu, Feb 9, 2017 at 10:08 PM, John Beard <john.j.beard@xxxxxxxxx> wrote:
>>>>>>>>>> Hi Wayne,
>>>>>>>>>>
>>>>>>>>>> Yes, first in the list is "8f192fe12 OPENGL_GAL: Init currentTarget"
>>>>>>>>>>
>>>>>>>>>> That particular one is actually an older patch I had on my tidy-up
>>>>>>>>>> branch and isn't really in the common.h set, but it keeps my IDE
>>>>>>>>>> quieter! 5e6a022b5 is the first common.h one.
>>>>>>>>>>
>>>>>>>>>> Cheers,
>>>>>>>>>>
>>>>>>>>>> John
>>>>>>>>>>
>>>>>>>>>> On Thu, Feb 9, 2017 at 9:56 PM, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>>>>>>>>>>> John,
>>>>>>>>>>>
>>>>>>>>>>> I like this idea.  Anything that prevents unnecessary compiling is a
>>>>>>>>>>> good thing.  Looking at this repo, I'm guessing that everything from
>>>>>>>>>>> commit 8f192fe12eb52efd6d024c82d1416db5e62b345f on is what needs to be
>>>>>>>>>>> merged.  I'll try to take a look at it over the weekend.  Keep up the
>>>>>>>>>>> great work.
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>>
>>>>>>>>>>> Wayne
>>>>>>>>>>>
>>>>>>>>>>> On 2/9/2017 8:44 AM, John Beard wrote:
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> I have a big set of patches on a branch that are mostly to do with
>>>>>>>>>>>> stripping seldom-used functions out of common.h into more targeted
>>>>>>>>>>>> headers:
>>>>>>>>>>>>
>>>>>>>>>>>> https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/tidy_ups
>>>>>>>>>>>>
>>>>>>>>>>>> The idea here is to reduce the visibility of these functions from
>>>>>>>>>>>> almost every file in KiCad to just those that need them. This also
>>>>>>>>>>>> means that changing the signatures (or even just adding comments)
>>>>>>>>>>>> doesn't force a huge recompile of the whole codebase, just the files
>>>>>>>>>>>> that use them. It also makes common.h less of a "dumping ground" for
>>>>>>>>>>>> random stuff.
>>>>>>>>>>>>
>>>>>>>>>>>> There were several old functions or variables that weren't used,
>>>>>>>>>>>> weren't defined, or weren't declared too, and these were tidied up.
>>>>>>>>>>>>
>>>>>>>>>>>> I have used std::unique_ptr in a couple of places, at one was leaking
>>>>>>>>>>>> an object before.
>>>>>>>>>>>>
>>>>>>>>>>>> I only meant to do a few, but since any big work in common.h is as bad
>>>>>>>>>>>> as small work, I did quite a few.
>>>>>>>>>>>>
>>>>>>>>>>>> The patches should mostly be cherry-pickable in case not all of them are wanted.
>>>>>>>>>>>>
>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>
>>>>>>>>>>>> John
>>>>>>>>>>>>
>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>> Mailing list: https://launchpad.net/~kicad-developers
>>>>>>>>>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>>>>>>>>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>>>>>>>>>> More help   : https://help.launchpad.net/ListHelp
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> _______________________________________________
>>>>>>>>>>> Mailing list: https://launchpad.net/~kicad-developers
>>>>>>>>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>>>>>>>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>>>>>>>>> More help   : https://help.launchpad.net/ListHelp
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> _______________________________________________
>>>>>>>>>>> Mailing list: https://launchpad.net/~kicad-developers
>>>>>>>>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>>>>>>>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>>>>>>>>> More help   : https://help.launchpad.net/ListHelp
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Mailing list: https://launchpad.net/~kicad-developers
>>>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>>>> More help   : https://help.launchpad.net/ListHelp
>>>>>>
>>>>
>>>> _______________________________________________
>>>> Mailing list: https://launchpad.net/~kicad-developers
>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>> More help   : https://help.launchpad.net/ListHelp
>>



References