D29008: Use the mount Pier Side property to manage Pier Flip
Chris Rowland
noreply at phabricator.kde.org
Thu Apr 23 12:40:49 BST 2020
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20200423/7541f8c8/attachment.html>
More information about the kde-edu
mailing list