[Marble-devel] Review Request 122686: Adds a right click menu to the statusbar that allows the user to quickly change the Angle display unit

[Calin Cruceru]

> On Feb. 23, 2015, 4:09 p.m., Calin Cruceru wrote:
> > src/apps/marble-qt/QtMainWindow.cpp, line 427
> > <https://git.reviewboard.kde.org/r/122686/diff/1/?file=350978#file350978line427>
> >
> >     No need for a space before the open parenthesis in any function call (including constructors). This should be:
> >     
> >     **new QAction( tr( "Degree (DMS)" ), statusBar() );**
> >     
> >     Also, make sure you do the same below and everywhere this pattern applies.

There are still lines which do not follow this convention.


> On Feb. 23, 2015, 4:09 p.m., Calin Cruceru wrote:
> > src/apps/marble-qt/QtMainWindow.cpp, line 430
> > <https://git.reviewboard.kde.org/r/122686/diff/1/?file=350978#file350978line430>
> >
> >     A space after the open paranthesis and before the closed parenthesis.
> >     
> >     **m_dmsDegreeAction->setData( Marble::DMSDegree );**
> >     
> >     Also, make sure you do the same below and everywhere this pattern applies.

There are still lines which do not follow this convention.


> On Feb. 23, 2015, 4:09 p.m., Calin Cruceru wrote:
> > src/apps/marble-qt/QtMainWindow.cpp, line 1059
> > <https://git.reviewboard.kde.org/r/122686/diff/1/?file=350978#file350978line1059>
> >
> >     See Qt's coding style regarding the open brace.
> >     
> >     Also, characteristic to Marble's coding style, leave one space after the open parenthesis and before the closed one. Everywhere.
> >     
> >     This is not always respected in Marble's code base . Maybe we could fix that in the future. Consistency is very important. :-)

You haven't repaired this as well.


> On Feb. 23, 2015, 4:09 p.m., Calin Cruceru wrote:
> > src/apps/marble-qt/QtMainWindow.cpp, line 1066
> > <https://git.reviewboard.kde.org/r/122686/diff/1/?file=350978#file350978line1066>
> >
> >     Spaces. Should be:
> >     
> >     **switch ( m_configDialog->angleUnit() ) {**
> >     
> >     Note that the space after the switch keyword and before the open brace is Qt specific. The only thing Marble adds to this is the two spaces before and after the expression within the switch statement.

You still need to add one more space after the switch keyword.


- Calin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122686/#review76472
-----------------------------------------------------------


On Feb. 25, 2015, 10:41 a.m., Marius Stanciu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122686/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2015, 10:41 a.m.)
> 
> 
> Review request for Marble.
> 
> 
> Bugs: 344364
>     http://bugs.kde.org/show_bug.cgi?id=344364
> 
> 
> Repository: marble
> 
> 
> Description
> -------
> 
> Menu contains degree(dms), degree(decimal) and utm formats, just like the settings dialog does.
> 
> 
> Diffs
> -----
> 
>   src/apps/marble-qt/QtMainWindow.h 595ffd2 
>   src/apps/marble-qt/QtMainWindow.cpp c4280c6 
>   src/lib/marble/QtMarbleConfigDialog.h 1fbee43 
>   src/lib/marble/QtMarbleConfigDialog.cpp ed5b62c 
> 
> Diff: https://git.reviewboard.kde.org/r/122686/diff/
> 
> 
> Testing
> -------
> 
> it works on my machine.
> 
> 
> Thanks,
> 
> Marius Stanciu
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20150225/bfceff61/attachment-0001.html>