← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] common.h tidyups

 

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
> 


Follow ups

References