D22954: Rolloff roof handling introduced

Jasem Mutlaq noreply at phabricator.kde.org
Tue Aug 6 07:43:16 BST 2019


mutlaqja added a comment.


  Great work Wolfgang! I have a rolloff roof so I will be definitely live testing this very soon!
  
  Please make the necessary changes as per the comments.

INLINE COMMENTS

> ksutils.cpp:1042
> +                        Options::schedulerLogging() ? "true" : "false")
> +            .append(QString("org.kde.kstars.ekos.observatory.debug=%2\n"
> +                            "org.kde.kstars.debug=%1").arg(

Why is this here and not with the rest of the modules above?

> dome.cpp:101-102
> +    // a rolloff roof is a dome that can move neither absolutely nor relatively
> +    if (currentDome)
> +        return (!currentDome->canAbsMove() && !currentDome->canRelMove());
> +

This function could be simplified to one line:

return (currentDome && !currentDome->canAbsMove() && !currentDome->canRelMove()))

> opslogs.ui:408
> +      <item row="8" column="2" colspan="4">
> +       <widget class="QCheckBox" name="kcfg_ObservatoryLogging">
> +        <property name="text">

missing tooltip

> observatory.cpp:266
>          case ISD::Dome::DOME_PARKING:
> -            domePark->setText(i18n("Parking"));
> -            domeUnpark->setText(i18n("UnPark"));
> -            enableMotionControl(false);
> +            toggleButtons(domePark, "Parking", domeUnpark, "Unpark");
> +            motionCWButton->setEnabled(true);

i18n

> observatory.cpp:270
> +            if (getDomeModel()->isRolloffRoof())
> +                domeAzimuthPosition->setText(i18n("CLOSING"));
> +            else

CLOSING --> Closing...

> observatory.cpp:281
>          case ISD::Dome::DOME_UNPARKING:
> -            domePark->setText(i18n("Park"));
> -            domeUnpark->setText(i18n("UnParking"));
> -            enableMotionControl(false);
> +            toggleButtons(domeUnpark, "Unparking", domePark, "Park");
> +            motionCCWButton->setEnabled(true);

i18n?

> observatory.cpp:285
> +            if (getDomeModel()->isRolloffRoof())
> +                domeAzimuthPosition->setText(i18n("OPENING"));
> +            else

Ditto

> observatory.cpp:317
> +        if (getDomeModel()->isRolloffRoof())
> +            domeAzimuthPosition->setText(i18n("OPEN"));
> +        else

OPEN --> Open

No need to capitalize unless extremely necessary for dangerous situations like CAUTION or something like that.

> observatory.cpp:565
> +    buttonCounterpart->setCheckable(false);
> +    buttonCounterpart->setText(i18n(titleCounterpart));
> +}

No need to use i18n here, the i18n should be used on string literals i18n("Foo") and not on variables.

So this should be setText(titleCounterpart) and the calling function should use the i18n

REPOSITORY
  R321 KStars

REVISION DETAIL
  https://phabricator.kde.org/D22954

To: wreissenberger, mutlaqja
Cc: kde-edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20190806/4703430b/attachment-0001.html>


More information about the kde-edu mailing list