kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #28311
Re: [Patch] Add an option to select a reference point and an anchor in pcbnew move exactly dialog
-
To:
Tomasz Wlostowski <tomasz.wlostowski@xxxxxxx>
-
From:
Robbert Lagerweij <rlagerweij@xxxxxxxxxxx>
-
Date:
Tue, 28 Feb 2017 13:00:16 +0000
-
Accept-language:
en-US
-
Authentication-results:
gmail.com; dkim=none (message not signed) header.d=none;gmail.com; dmarc=none action=none header.from=hotmail.com;
-
Cc:
KiCad Developers <kicad-developers@xxxxxxxxxxxxxxxxxxx>
-
In-reply-to:
<f0afa069-d9a2-3858-a781-cf83232c0914@cern.ch>
-
Spamdiagnosticmetadata:
NSPM
-
Spamdiagnosticoutput:
1:99
-
Thread-index:
AQHSkFnQN/3S7VYk50mbdq079zGGu6F8qSGAgACaoDWAAP5PgIAAInCo
-
Thread-topic:
[Kicad-developers] [Patch] Add an option to select a reference point and an anchor in pcbnew move exactly dialog
Hi Thomas,
Thank you for your feedback. The duplication of code is indeed not how I would usually approach this but I thought it the lesser of two evils given the fact that the legacy stuff will most likely be deprecated immediately after the 5.0 release. This means that long term maintainability is not really affected since there will be limited chance of structural changes needed to that part of the code (presumably only 5.x bug fixes) . The duplication does have the benefit of allowing the legacy users to use this functionality as well.
If we go the GAL only route, since legacy and GAL use the same dialog, I could either create a new dialog which we only use in GAL or add additional logic to disable/hide the functionality in legacy.
I'm happy to hear your opinion (and anyone else's for that matter) on the best way forward.
I'll check the other issues you mentioned.
Kind regards,
Robbert
Get Outlook for Android<https://aka.ms/ghei36>
________________________________
From: Tomasz Wlostowski <tomasz.wlostowski@xxxxxxx>
Sent: Tuesday, February 28, 2017 11:56:59 AM
To: Robbert Lagerweij
Cc: firewalker; KiCad Developers
Subject: Re: [Kicad-developers] [Patch] Add an option to select a reference point and an anchor in pcbnew move exactly dialog
On 27.02.2017 20:50, Robbert Lagerweij wrote:
> This is a corrected version of the patch.
>
>
> My apologies again for letting this slip through.
>
> To be doubly sure, I just performed a merge and build test on this patch
> and it should apply cleanly to master.
>
Hi Robert,
I had a look at your patch, there seems to be a lot of repetitive code
(the functionality is duplicated between legacy/GAL canvas). Personally
I wouldn't add new features to the legacy canvas. I also noticed you use
a bitwise | operator instead of the logical one in few places in the
code and some minor coding style issues (e.g. naming of new members in
DIALOG_MOVE_EXACT class). Could you have a look at these issues?
Thank you very much for your contribution!
Tom
Follow ups
References
