kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

Fabrizio,

I pushed your patch to the master branch.  Thank you for your
contribution to KiCad.

This is not aimed at your patch but one thing I notice is that there is
and inconsistency in some of our wxStaticBoxSizer() strings.  There are
a few cases where the string ends with a colon (:) but it's rare.  For
the sake of consistency, if you find any cases in the future where this
is true, please remove the trailing colon.

Cheers,

Wayne


On 7/20/2017 5:59 AM, Fabrizio Tappero wrote:
> Hi Wayne,
> you are right. Sorry about that. I have reviewed and changed all instances. 
> 
> In attachment the new patch.
> 
> cheers
> Fabrizio
> 
> 
> On Wed, Jul 19, 2017 at 3:59 PM, Wayne Stambaugh <stambaughw@xxxxxxxxx
> <mailto:stambaughw@xxxxxxxxx>> wrote:
> 
>     Fabrizio,
> 
>     You are still using header instead of sentence capitalization of text
>     box labels.  For example, in pcbnew/dialogs/dialog_plot_base.cpp you
>     changed:
> 
>     "Plot format:"
> 
>     to
> 
>     "Plot Format:"
> 
>     The original capitalization is correct.  I see a lot of this in this
>     patch.
> 
>     Please correct the text box label capitalization and resubmit your
>     patch.  Also, please give your patches a unique name so I don't have to.
>      Even easier, just use the name generated by `git format-patch`.
> 
>     Wayne
> 
>     On 7/19/2017 6:55 AM, Fabrizio Tappero wrote:
>     > Hi Wayne,
>     > really tedious work to spot these kind of mistakes... I hope I fixed all
>     > mistakes.
>     >
>     > In attachment the new patch.
>     >
>     > cheers
>     > Fabrizio
>     >
>     >
>     > On Wed, Jul 19, 2017 at 12:19 PM, Fabrizio Tappero
>     > <fabrizio.tappero@xxxxxxxxx <mailto:fabrizio.tappero@xxxxxxxxx>
>     <mailto:fabrizio.tappero@xxxxxxxxx
>     <mailto:fabrizio.tappero@xxxxxxxxx>>> wrote:
>     >
>     >     Hi Wayne,
>     >     apologies for that. I sometimes think that a unique patch is more
>     >     friendly than several ones.
>     >
>     >     Will fix and resubmit.
>     >
>     >     cheers
>     >     Fabrizio
>     >
>     >
>     >     On Tue, Jul 18, 2017 at 3:35 PM, Wayne Stambaugh
>     >     <stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>
>     <mailto:stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>>> wrote:
>     >
>     >         Fabrizio,
>     >
>     >         This patch has far more changes than the plot dialogs
>     mentioned
>     >         in your
>     >         commit message.  In the future, please either make your commit
>     >         message
>     >         describe all of the changes related to the patch.  I would
>     >         prefer that
>     >         you only make one wxFormbuilder dialog change per patch.
>     >         wxFormbuilder
>     >         changes tend to be noisy so it takes longer to review them.
>     >
>     >         You are using the incorrect capitalization in text box labels.
>     >         Text box
>     >         labels use sentence capitalization not header
>     capitalization per
>     >         the UI
>     >         guidelines[1].
>     >
>     >         Please fix your patch accordingly so I can commit it.
>     >
>     >         Cheers,
>     >
>     >         Wayne
>     >
>     >       
>      [1]:http://docs.kicad-pcb.org/doxygen/md_Documentation_development_ui-policy.html
>     <http://docs.kicad-pcb.org/doxygen/md_Documentation_development_ui-policy.html>
>     >       
>      <http://docs.kicad-pcb.org/doxygen/md_Documentation_development_ui-policy.html
>     <http://docs.kicad-pcb.org/doxygen/md_Documentation_development_ui-policy.html>>
>     >
>     >         On 7/3/2017 4:45 AM, Fabrizio Tappero wrote:
>     >         > Hi JP,
>     >         > thanks for it. I updated my wxformbuilder to the latest
>     (3.6v)
>     >         > version. And I fixed the (mm) issue you mentioned.
>     >         >
>     >         > In attachment the patch.
>     >         >
>     >         > thank you
>     >         > Fabrizio
>     >         >
>     >         >
>     >         > On Sat, Jul 1, 2017 at 8:40 PM, jp charras
>     >         <jp.charras@xxxxxxxxxx <mailto:jp.charras@xxxxxxxxxx>
>     <mailto:jp.charras@xxxxxxxxxx <mailto:jp.charras@xxxxxxxxxx>>> wrote:
>     >         >> Le 30/06/2017 à 15:34, Fabrizio Tappero a écrit :
>     >         >>> Hello,
>     >         >>> in attachment the patch reviewed after Wayne comments.
>     >         >>>
>     >         >>> Best regards
>     >         >>> Fabrizio
>     >         >>>
>     >         >>
>     >         >> Hi Fabrizio,
>     >         >>
>     >         >> I had a look at your last patch, and found a few issues:
>     >         >> for instance the change:
>     >         >>
>     >         >> -       m_defaultLineWidthTitle = new wxStaticText(
>     >         sbSizerPlotFormat->GetStaticBox(), wxID_ANY,
>     >         >> _("Default line thickness"), wxDefaultPosition,
>     >         wxDefaultSize, 0 );
>     >         >> +       m_defaultLineWidthTitle = new wxStaticText(
>     >         sbSizerPlotFormat->GetStaticBox(), wxID_ANY, _("Line
>     >         >> Thickness (mm):"), wxDefaultPosition, wxDefaultSize, 0 );
>     >         >>
>     >         >>
>     >         >> is not good for 2 reasons:
>     >         >> - "Default line thickness" is the right title (see a
>     previous
>     >         Wayne's answer)
>     >         >> - "Line Thickness (mm):" is incorrect:
>     >         >> the units symbol " (mm):" is added a run time, because
>     units
>     >         can be mm or inches.
>     >         >> so never change titles like "Value" to "Value (mm):".
>     >         >> It creates a wrong title.
>     >         >>
>     >         >> Besides your wxFormBuilder version is rather old and
>     creates
>     >         incorrect C++ sources: please update it.
>     >         >>
>     >         >> Thanks.
>     >         >>
>     >         >> --
>     >         >> Jean-Pierre CHARRAS
>     >         >>
>     >         >>
>     >         >> _______________________________________________
>     >         >> Mailing list: https://launchpad.net/~kicad-developers
>     <https://launchpad.net/~kicad-developers>
>     >         <https://launchpad.net/~kicad-developers
>     <https://launchpad.net/~kicad-developers>>
>     >         >> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>     >         <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx
>     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>>
>     >         >> Unsubscribe : https://launchpad.net/~kicad-developers
>     <https://launchpad.net/~kicad-developers>
>     >         <https://launchpad.net/~kicad-developers
>     <https://launchpad.net/~kicad-developers>>
>     >         >> More help   : https://help.launchpad.net/ListHelp
>     <https://help.launchpad.net/ListHelp>
>     >         <https://help.launchpad.net/ListHelp
>     <https://help.launchpad.net/ListHelp>>
>     >
>     >         _______________________________________________
>     >         Mailing list: https://launchpad.net/~kicad-developers
>     <https://launchpad.net/~kicad-developers>
>     >         <https://launchpad.net/~kicad-developers
>     <https://launchpad.net/~kicad-developers>>
>     >         Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>     >         <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx
>     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>>
>     >         Unsubscribe : https://launchpad.net/~kicad-developers
>     <https://launchpad.net/~kicad-developers>
>     >         <https://launchpad.net/~kicad-developers
>     <https://launchpad.net/~kicad-developers>>
>     >         More help   : https://help.launchpad.net/ListHelp
>     <https://help.launchpad.net/ListHelp>
>     >         <https://help.launchpad.net/ListHelp
>     <https://help.launchpad.net/ListHelp>>
>     >
>     >
>     >
> 
>