kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #30055
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
-
Re: [PATCH] correct text inside two importantplot windows
From: liyoubdu, 2017-06-08
-
Re: [PATCH] correct text inside two importantplot windows
From: Fabrizio Tappero, 2017-06-09
-
Re: [PATCH] correct text inside two importantplot windows
From: Wayne Stambaugh, 2017-06-21
-
Re: [PATCH] correct text inside two importantplot windows
From: Diego Herranz, 2017-06-21
-
Re: [PATCH] correct text inside two importantplot windows
From: Fabrizio Tappero, 2017-06-22
-
Re: [PATCH] correct text inside two importantplot windows
From: Wayne Stambaugh, 2017-06-23
-
Re: [PATCH] correct text inside two importantplot windows
From: Fabrizio Tappero, 2017-06-26
-
Re: [PATCH] correct text inside two importantplot windows
From: Fabrizio Tappero, 2017-06-30
-
Re: [PATCH] correct text inside two importantplot windows
From: jp charras, 2017-07-01
-
Re: [PATCH] correct text inside two importantplot windows
From: Fabrizio Tappero, 2017-07-03
-
Re: [PATCH] correct text inside two importantplot windows
From: Wayne Stambaugh, 2017-07-18
-
Re: [PATCH] correct text inside two importantplot windows
From: Fabrizio Tappero, 2017-07-19
-
Re: [PATCH] correct text inside two importantplot windows
From: Fabrizio Tappero, 2017-07-19