[PATCH] Make Marble optional

Anton Lundin glance at acc.umu.se
Tue Mar 4 15:34:33 PST 2014


On 04 March, 2014 - Alberto Corona wrote:

> Right, first off I fixed the warnings by the compiler by returning the
> object and event. I also fixed the qmake config (it was previously
> completely broken). Also added the instructions to compile without in
> the INSTALL.
> 

First, There are nothing about attribution about where most of the code
comes from. If you base your code on my code, write that in the commit
message. Add also a Signed-off-by: from me as the first of the lines in
the "audit-trail".
When you continue on someone else work its a really good ask that person
for a ACK that it looks sane. 

For example when Dirk sees that you have bin continuing on something i
wrote, he usually would like my feedback before merging that code.

https://www.kernel.org/doc/Documentation/SubmittingPatches have a quite
good explanation how to think surrounding how and when stuff should be
signed.


If we don't completely remove that frame from the main window it should
be replaced with something saying that marble is disabled at least.


Then the "LIBS -= -lmarblewidget", if you look up a couple of rows, it
might be named -lmarblewidgetd .


Could you elaborate on the: "QSettings
had to be included into maintab.h in order for QSettings to be defined."

I can't find any reason for that change to interact with the rest of the
code.


//Anton


> On Tue, Mar 04, 2014 at 09:33:13AM +0100, Anton Lundin wrote:
> > On 03 March, 2014 - Alberto Corona wrote:
> > 
> > > Here's the link again as I forgot to include the list
> > > https://github.com/0x1A/subsurface/commit/eb215101097120f8c62339fd767b71574cdd0bbd
> > > 
> > > Like Anton said, this is a bit hacky, though I don't think there's a very
> > > easy way to do so without reworking quite a bit of the mainwindow (for
> > > which I'm too inexperienced with Qt for to change).
> > > 
> > 
> > When resubmitting a patch, its a god practice to resubmit it in the same
> > way as you submitted your first patch.
> > 
> > Its also a really god idea to include a changelog addressing the review
> > comments from the last submission.
> > 
> > 
> > So, pleas go back and look at the comments from the last submission and
> > tell us how you have addressed those.
> > 
> > 
> > //Anton
> > 
> > 
> > > On Mon, Mar 3, 2014 at 8:00 PM, Alberto Corona <albcoron at gmail.com> wrote:
> > > 
> > > >
> > > > The changes are here
> > > > https://github.com/0x1A/subsurface/commit/eb215101097120f8c62339fd767b71574cdd0bbd
> > > >
> > > >
> > > > On Mon, Mar 3, 2014 at 7:29 PM, Dirk Hohndel <dirk at hohndel.org> wrote:
> > > >
> > > >> On Mon, 2014-03-03 at 18:17 -0600, Alberto Corona wrote:
> > > >> > Ok so I've got things working now, though I'd like to see if Anton is
> > > >> > ok with the changes. Unfortunately, I'm not familiar enough with Qt to
> > > >> > change the way the splitters work in order to get rid of the blank
> > > >> > dummy widget.
> > > >> >
> > > >>
> > > >> And where would we find those changes?
> > > >>
> > > >> /D
> > > >>
> > > >>
> > > >>
> > > >
> > 
> > > _______________________________________________
> > > subsurface mailing list
> > > subsurface at hohndel.org
> > > http://lists.hohndel.org/cgi-bin/mailman/listinfo/subsurface
> > 
> > 
> > -- 
> > Anton Lundin	+46702-161604

> >From eb215101097120f8c62339fd767b71574cdd0bbd Mon Sep 17 00:00:00 2001
> From: Alberto Corona <albcoron at gmail.com>
> Date: Mon, 3 Mar 2014 13:49:21 -0600
> Subject: [PATCH] Make Marble optional at compile
> 
> Make Marble optional at compile by creating a dummy widget. QSettings
> had to be included into maintab.h in order for QSettings to be defined.
> Add instructions in INSTALL for compiling without Marble.
> 
> Signed-off-by: Alberto Corona <albcoron at gmail.com>
> ---
>  INSTALL                  |  1 +
>  qt-ui/globe.cpp          | 12 ++++++++++++
>  qt-ui/globe.h            | 21 +++++++++++++++++++++
>  qt-ui/maintab.h          |  1 +
>  subsurface-configure.pri |  6 ++++++
>  5 files changed, 41 insertions(+)
> 
> diff --git a/INSTALL b/INSTALL
> index be3c5ed..0c192e6 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -74,6 +74,7 @@ $ sudo make install
>  To compile Subsurface:
>  $ git clone git://subsurface.hohndel.org/subsurface.git
>  $ cd subsurface
> + - You can use "CONFIG += no-marble" to compile without the Marble widget -
>  $ qmake # qmake-qt4 on some flavors of Linux
>  $ make
>  $ sudo make install     [optionally, add: prefix=/usr/local]
> diff --git a/qt-ui/globe.cpp b/qt-ui/globe.cpp
> index 496afb4..69a12c5 100644
> --- a/qt-ui/globe.cpp
> +++ b/qt-ui/globe.cpp
> @@ -1,4 +1,5 @@
>  #include "globe.h"
> +#ifndef NO_MARBLE
>  #include "kmessagewidget.h"
>  #include "mainwindow.h"
>  #include "ui_mainwindow.h"
> @@ -297,3 +298,14 @@ void GlobeGPS::resizeEvent(QResizeEvent *event)
>  		messageWidget->setGeometry(5, 5, size - 10, 0);
>  	messageWidget->setMaximumHeight(500);
>  }
> +
> +#else
> +
> +GlobeGPS::GlobeGPS(QWidget *parent) {}
> +void GlobeGPS::reload() {}
> +void GlobeGPS::repopulateLabels() {}
> +void GlobeGPS::centerOn(struct dive *dive) {}
> +void GlobeGPS::prepareForGetDiveCoordinates() {}
> +bool GlobeGPS::eventFilter(QObject *obj, QEvent *ev) {return QObject::eventFilter(obj, ev);}
> +
> +#endif
> diff --git a/qt-ui/globe.h b/qt-ui/globe.h
> index 80d9613..fb57d2e 100644
> --- a/qt-ui/globe.h
> +++ b/qt-ui/globe.h
> @@ -1,5 +1,6 @@
>  #ifndef GLOBE_H
>  #define GLOBE_H
> +#ifndef NO_MARBLE
>  
>  #include <marble/MarbleWidget.h>
>  #include <marble/GeoDataCoordinates.h>
> @@ -41,4 +42,24 @@ slots:
>  	void prepareForGetDiveCoordinates();
>  };
>  
> +#else
> +
> +// Dummy widget in place of Marble widget
> +
> +#include <QWidget>
> +
> +class GlobeGPS : public QWidget {
> +	Q_OBJECT
> +public:
> +	GlobeGPS(QWidget *parent);
> +	void reload();
> +	void repopulateLabels();
> +	void centerOn(struct dive *dive);
> +	bool eventFilter(QObject *, QEvent *);
> +public
> +slots:
> +	void prepareForGetDiveCoordinates();
> +};
> +
> +#endif
>  #endif // GLOBE_H
> diff --git a/qt-ui/maintab.h b/qt-ui/maintab.h
> index 946b673..fda74dc 100644
> --- a/qt-ui/maintab.h
> +++ b/qt-ui/maintab.h
> @@ -9,6 +9,7 @@
>  
>  #include <QTabWidget>
>  #include <QDialog>
> +#include <QSettings>
>  #include <QMap>
>  
>  #include "models.h"
> diff --git a/subsurface-configure.pri b/subsurface-configure.pri
> index 8e6aead..8d38934 100644
> --- a/subsurface-configure.pri
> +++ b/subsurface-configure.pri
> @@ -148,6 +148,12 @@ win32 {
>  	DEFINES -= UNICODE
>  }
>  
> +# Optional Marble
> +no-marble {
> +	DEFINES += NO_MARBLE
> +	LIBS -= -lmarblewidget
> +}
> +
>  #
>  # misc
>  #
> -- 
> 1.9.0
> 


-- 
Anton Lundin	+46702-161604


More information about the subsurface mailing list