D29008: Use the mount Pier Side property to manage Pier Flip

chrisrowland marked 4 inline comments as done.
chrisrowland added a comment.


  My first attempt to add inline comments failed, try again.

INLINE COMMENTS

> TallFurryMan wrote in capture.cpp:6831
> It is getting cast of course, but my point is that the function has no reason to return a QString when what it returns is a pooled char pointer. When the caller function wants to use the raw string (see QTest's fixture identifiers for instance) it must re-convert the QString back to StdString and retrieve the C internal pointer to the array. Lazy conversion is always best.

This is my C# .NET background coming out, I don't see a problem with returning a string as what it is - a string.

Not sure what would be a more friendly name, MeridianFlipStagestring() maybe?

> TallFurryMan wrote in mount.cpp:604
> std::numeric <double>::max maybe?
> But that would make the log weirder than it already is when pierSide() doesn't report a useful value, so OK.
> Or duplicate your log inside the switch/case and drop the variable.

No change required, certainly not going to duplicate the log statement three times, there's too much copy and paste code anyway.

> TallFurryMan wrote in mount.cpp:1137
> This function misses translations.

what do you mean?  If to different languages I have no idea how to do this.

> TallFurryMan wrote in mount.cpp:1257
> Pedantic, you test "other than PIER_WEST and PIER_EAST" elsewhere, and PIER_UNKNOWN here.

By design, a mount that does not implement pier side will return PIER_UNKNOWN, that's the only other value in the enum.

I don't want a mount that doesn't report pier side to fail because it will keep retrying, it needs to behave as now where it tries once only.

> TallFurryMan wrote in mount.cpp:1343
> This must use the KStars simulation time, not the system time.

Not sure what the various times available to me are, a clue would help.

> TallFurryMan wrote in mount.ui:371
> This should not move I think.

Are these changes in the UI under my control? the sizes maybe but the auto generated code moving isn't something I can control.

REPOSITORY
  R321 KStars

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

To: chrisrowland, mutlaqja
Cc: TallFurryMan, wreissenberger, kde-edu, narvaez, apol