[Marble-devel] Review Request: Marble Clock Patch

hjain.itbhu at gmail.com hjain.itbhu at gmail.com
Sun Aug 8 23:18:05 CEST 2010 


> On 2010-08-04 10:52:09, Dennis Nienhüser wrote:
> > /trunk/KDE/kdeedu/marble/src/lib/MarbleClock.h, line 60
> > <http://reviewboard.kde.org/r/4819/diff/2/?file=32183#file32183line60>
> >
> >     what effect does the speed have? Is it an acceleration factor (see above)?
> 
> hjain wrote:
>     The speed of timer is how fast the marble clock can run relative to actual speed of time. Value of speed may varies form -10x to 10x ( 10x means)
> 
> Bastian Holst wrote:
>     Could you please add an explanation in the doxygen comment (still not your code).

fixed it.


> On 2010-08-04 10:52:09, Dennis Nienhüser wrote:
> > /trunk/KDE/kdeedu/marble/src/lib/MarbleClock.h, line 72
> > <http://reviewboard.kde.org/r/4819/diff/2/?file=32183#file32183line72>
> >
> >     As a difference, i guess? To which timezone? UTC? Maybe setTimezoneDifference() and timezoneDifference() would be better method names
> >
> 
> hjain wrote:
>     Yes, it is difference to UTC. I agree that setTimezoneDifference() and timezoneDifference() are more specific names but generally we talk the timezone w.r.t. UTC so setTimezone() and timezone() are more reable.
> 
> Bastian Holst wrote:
>     Same here.

fixed it.


> On 2010-08-04 10:52:09, Dennis Nienhüser wrote:
> > /trunk/KDE/kdeedu/marble/src/lib/MarbleClock.h, line 43
> > <http://reviewboard.kde.org/r/4819/diff/2/?file=32183#file32183line43>
> >
> >     doxygen please - when does it get emitted? every second? when i call setDataTime?
> 
> hjain wrote:
>     timeChanged() signal is called every minute( according to marble clock ) and when setDateTime() is called.
> 
> Bastian Holst wrote:
>     Please add a doxygen documentation comment explaining that.

I don't think this is really good idea because emitting of signal is dependent on some other function.


> On 2010-08-04 10:52:09, Dennis Nienhüser wrote:
> > /trunk/KDE/kdeedu/marble/src/marble_part.cpp, line 839
> > <http://reviewboard.kde.org/r/4819/diff/2/?file=32203#file32203line839>
> >
> >     Better copy the icon and rename it
> 
> hjain wrote:
>     The oxygen theme contains icons of different sizes ranging from 16x16 to 64x64 pixels. How will be renamed icon will be added to oxygen theme?
> 
> Bastian Holst wrote:
>     Have you done this already?

not yet. This can be done at some later stage along with idea to create two different separate features:- 1) Position where Sun is at zenith and 2) Center at Sun Position.


- hjain


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


On 2010-08-07 14:15:05, hjain wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4819/
> -----------------------------------------------------------
> 
> (Updated 2010-08-07 14:15:05)
> 
> 
> Review request for marble and Bastian Holst.
> 
> 
> Summary
> -------
> 
> This patch has the following features:-
> 1) The older 'Sun Control' dialog box is broken into new 'Sun Control' dialog box and 'Time Control' dialog box.
> 2) The 'Configure Marble Desktop Globe' has 'Date and Time' tab which has configuration features for time.
> 3) The current time is shown in status bar.
> 4) The icon of the Sun is shown on the Earth for zenith feature in 'Sun Control' dialog box.
> 5) Toolbar action button are added for shadow, night map and zenith features(although shadow and night map toolbar buttons are disabled in this patch because of lack of appropriate icons for them).
> 6) The name of ExtDateTime class is changed to MarbleClock. The license of David Roberts(author of ExtDateTime class) has also been updated for this class with this permissions.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdeedu/marble/src/QtMainWindow.h 1157379 
>   /trunk/KDE/kdeedu/marble/src/QtMainWindow.cpp 1157379 
>   /trunk/KDE/kdeedu/marble/src/bindings/python/sip/MarbleModel.sip 1157379 
>   /trunk/KDE/kdeedu/marble/src/bindings/python/sip/SunLocator.sip 1157379 
>   /trunk/KDE/kdeedu/marble/src/lib/CMakeLists.txt 1157379 
>   /trunk/KDE/kdeedu/marble/src/lib/ExtDateTime.h 1157379 
>   /trunk/KDE/kdeedu/marble/src/lib/ExtDateTime.cpp 1157379 
>   /trunk/KDE/kdeedu/marble/src/lib/LayerManager.cpp 1157379 
>   /trunk/KDE/kdeedu/marble/src/lib/MarbleClock.h PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/MarbleClock.cpp PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/MarbleDataFacade.cpp 1157379 
>   /trunk/KDE/kdeedu/marble/src/lib/MarbleModel.h 1157379 
>   /trunk/KDE/kdeedu/marble/src/lib/MarbleModel.cpp 1157379 
>   /trunk/KDE/kdeedu/marble/src/lib/MarbleTimeSettingsWidget.ui PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/QtMarbleConfigDialog.h 1157379 
>   /trunk/KDE/kdeedu/marble/src/lib/QtMarbleConfigDialog.cpp 1157379 
>   /trunk/KDE/kdeedu/marble/src/lib/SunControlWidget.h 1157379 
>   /trunk/KDE/kdeedu/marble/src/lib/SunControlWidget.cpp 1157379 
>   /trunk/KDE/kdeedu/marble/src/lib/SunControlWidget.ui 1157379 
>   /trunk/KDE/kdeedu/marble/src/lib/SunLocator.h 1157379 
>   /trunk/KDE/kdeedu/marble/src/lib/SunLocator.cpp 1157379 
>   /trunk/KDE/kdeedu/marble/src/lib/TimeControlWidget.h PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/TimeControlWidget.cpp PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/TimeControlWidget.ui PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/blendings/SunLightBlending.cpp 1157379 
>   /trunk/KDE/kdeedu/marble/src/lib/routing/AdjustNavigation.cpp 1157379 
>   /trunk/KDE/kdeedu/marble/src/lib/routing/RoutingManager.cpp 1157379 
>   /trunk/KDE/kdeedu/marble/src/marble.kcfg 1157379 
>   /trunk/KDE/kdeedu/marble/src/marble_part.h 1157379 
>   /trunk/KDE/kdeedu/marble/src/marble_part.cpp 1157379 
>   /trunk/KDE/kdeedu/marble/src/marble_part.rc 1157379 
>   /trunk/KDE/kdeedu/marble/src/marbleui.rc 1157379 
>   /trunk/KDE/kdeedu/marble/src/plugins/render/stars/StarsPlugin.h 1157379 
>   /trunk/KDE/kdeedu/marble/src/plugins/render/stars/StarsPlugin.cpp 1157379 
> 
> Diff: http://reviewboard.kde.org/r/4819/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> hjain
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/marble-devel/attachments/20100808/d0bb2f84/attachment-0001.htm

More information about the Marble-devel mailing list