← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] correct text inside two importantplot windows

 

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>> 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>> 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>
> 
>         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>> 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>
>         >> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>         <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>         >> Unsubscribe : https://launchpad.net/~kicad-developers
>         <https://launchpad.net/~kicad-developers>
>         >> More help   : https://help.launchpad.net/ListHelp
>         <https://help.launchpad.net/ListHelp>
> 
>         _______________________________________________
>         Mailing list: https://launchpad.net/~kicad-developers
>         <https://launchpad.net/~kicad-developers>
>         Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>         <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>         Unsubscribe : https://launchpad.net/~kicad-developers
>         <https://launchpad.net/~kicad-developers>
>         More help   : https://help.launchpad.net/ListHelp
>         <https://help.launchpad.net/ListHelp>
> 
> 
> 


Follow ups

References