kicad-developers team mailing list archive

[Fabrizio Tappero <fabrizio.tappero@xxxxxxxxx>]

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>
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
>
> 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>
> 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
> >> 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
>