[Kstars-devel] Review Request 112496: Column for Object 2 is added in conjunctions tool and default width of column is set larger

Rafal Kulaga rl.kulaga at gmail.com
Fri Sep 13 22:01:22 UTC 2013


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112496/#review39997
-----------------------------------------------------------


This is definitely a nice usability improvement. I have marked a few comments in the diff. Could you please look at them?

Thanks!


kstars/tools/conjunctions.h
<http://git.reviewboard.kde.org/r/112496/#comment29550>

    It would make sense to use a const reference here instead of passing those QStrings by value. Also, I would name those arguments object1 and object2, just to be consistent with the rest of the code.



kstars/tools/conjunctions.cpp
<http://git.reviewboard.kde.org/r/112496/#comment29559>

    A wild whitespace appeared after "object".



kstars/tools/conjunctions.cpp
<http://git.reviewboard.kde.org/r/112496/#comment29562>

    Again, WS issues.



kstars/tools/conjunctions.cpp
<http://git.reviewboard.kde.org/r/112496/#comment29565>

    Again, maybe using object1 would be more consistent.


- Rafal Kulaga


On Sept. 4, 2013, 8:11 p.m., Vijay Dhameliya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112496/
> -----------------------------------------------------------
> 
> (Updated Sept. 4, 2013, 8:11 p.m.)
> 
> 
> Review request for KStars and Akarsh Simha.
> 
> 
> Description
> -------
> 
> In conjunctions tool default width of column in text box is set large enough to make it look better and column "object 2" is added to keep log of to which object the object 1 is mapping,  when we have calculated many conjunctions from same object 1 to different object 2s.
> 
> 
> Diffs
> -----
> 
>   kstars/tools/conjunctions.h b51209e 
>   kstars/tools/conjunctions.cpp 39cef00 
>   kstars/tools/conjunctions.ui 80374b1 
> 
> Diff: http://git.reviewboard.kde.org/r/112496/diff/
> 
> 
> Testing
> -------
> 
> tested
> 
> 
> File Attachments
> ----------------
> 
> 
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/09/03/Conjunction_tool_after.png
> 
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/09/03/Conjunction_tool_Before.png
> 
> 
> Thanks,
> 
> Vijay Dhameliya
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kstars-devel/attachments/20130913/81f56c0e/attachment.html>


More information about the Kstars-devel mailing list