[Marble-devel] Re: Review Request: Add planet option to map creation wizard

[denis at xnapp.co.uk]

> On 2010-12-29 15:42:52, Torsten Rahn wrote:
> > /trunk/KDE/kdeedu/marble/src/lib/Planet.cpp, line 351
> > <http://svn.reviewboard.kde.org/r/6223/diff/3/?file=43268#file43268line351>
> >
> >     The implementation should be closer in sync with the one of the name method. Currently it would easily be possible to accidently have different string ids in both implementations, possibly contradicting each other. This makes maintenance harder and the implementation more prone to errors.

Would this not occur with the method 

Planet::Planet( const QString& id )
    : d( new PlanetPrivate )

as well. Also could you suggest how I could achieve this because a list of variables of viable planet IDs seems a bad way to do it. 


- den


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


On 2010-12-29 13:37:30, den wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6223/
> -----------------------------------------------------------
> 
> (Updated 2010-12-29 13:37:30)
> 
> 
> Review request for marble.
> 
> 
> Summary
> -------
> 
> Google Code In Task - http://socghop.appspot.com/gci/task/show/google/gci2010/kde/t129277012002
> 
> Adds a planet option combobox to the map creation wizard page.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdeedu/marble/src/lib/MapWizard.h 1210090 
>   /trunk/KDE/kdeedu/marble/src/lib/MapWizard.cpp 1210090 
>   /trunk/KDE/kdeedu/marble/src/lib/MapWizard.ui 1210090 
>   /trunk/KDE/kdeedu/marble/src/lib/Planet.h 1210090 
>   /trunk/KDE/kdeedu/marble/src/lib/Planet.cpp 1210090 
> 
> Diff: http://svn.reviewboard.kde.org/r/6223/diff
> 
> 
> Testing
> -------
> 
> 
> Screenshots
> -----------
> 
> 
>   http://svn.reviewboard.kde.org/r/6223/s/591/
> 
> 
> Thanks,
> 
> den
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/marble-devel/attachments/20101229/9b38204a/attachment.htm