Differential D3563

Added a warning when no plugins could be loaded, see T2114 for more details.
ClosedPublic
Actions

Authored by Rakete1111 on Dec 1 2016, 7:43 PM.

Details

Reviewers

nienhueser

Group Reviewers

Marble

Diff Detail

Repository

R34 Marble

Branch

master

Lint

No Linters Available

Unit

No Unit Test Coverage

Rakete1111 updated this revision to Diff 8685.Dec 1 2016, 7:43 PM

Rakete1111 retitled this revision from to Added a warning when no plugins could be loaded, see T2114 for more details..

Rakete1111 updated this object.

Rakete1111 edited the test plan for this revision. (Show Details)

Nice start :-) I guess that's a successful liftoff for our rocket!

src/lib/marble/PluginManager.cpp
290	Do we really want a QMessageBox here or would we rather want an OSD message?

rahn added inline comments.Dec 1 2016, 8:16 PM

src/lib/marble/PluginManager.cpp
286	.isEmpty()

Didn't know about isEmpty(), but should have, I feel bad now...
If by OSD you mean the notifications from KDE, I don't think it is necessary: Unix users can always look at the debug output, just Windows users would have to debug the application to get those messages, so that's why a QMessageBox is better for them I figure.

In D3563#66539, @Rakete1111 wrote:

Didn't know about isEmpty(), but should have, I feel bad now...
If by OSD you mean the notifications from KDE, I don't think it is necessary: Unix users can always look at the debug output, just Windows users would have to debug the application to get those messages, so that's why a QMessageBox is better for them I figure.

Makes sense to me.

nienhueser added a reviewer: Marble.Dec 1 2016, 9:01 PM

In D3563#66539, @Rakete1111 wrote:

Didn't know about isEmpty(), but should have, I feel bad now...
If by OSD you mean the notifications from KDE, I don't think it is necessary: Unix users can always look at the debug output, just Windows users would have to debug the application to get those messages, so that's why a QMessageBox is better for them I figure.

No, I meant a text on the screen that would quickly fade in and out.

In D3563#66543, @rahn wrote:

In D3563#66539, @Rakete1111 wrote:

Didn't know about isEmpty(), but should have, I feel bad now...
If by OSD you mean the notifications from KDE, I don't think it is necessary: Unix users can always look at the debug output, just Windows users would have to debug the application to get those messages, so that's why a QMessageBox is better for them I figure.

No, I meant a text on the screen that would quickly fade in and out.

I don't think that's necessary. The situation we address here is a broken installation, mostly meant to notify 3rd party developers or packagers about serious problems in the installed files.

Used isEmpty() instead of testing for equality with empty string.

Just changed the if condition to use isEmpty() instead of a explicit check against an empty string.

nienhueser accepted this revision.Dec 3 2016, 10:37 PM

nienhueser added a reviewer: nienhueser.

This revision is now accepted and ready to land.Dec 3 2016, 10:37 PM

Submitted

Revision Contents
Changeset List

			Path	Packages
M			src/lib/marble/PluginManager.cpp (22 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	8685	37eb1ba		Dec 1 2016, 7:43 PM	★	★
Diff 2	8690	37eb1ba	- Used isEmpty() instead of testing for equality with empty string.	Dec 2 2016, 5:18 AM	★	★

Commit	Tree	Parents	Author	Summary	Date
8a233404ca6f	c11fa15b417c	9c6192c0a615	Nicolas Lesser	Used isEmpty() instead of testing for equality with empty string.	Dec 2 2016, 5:16 AM
9c6192c0a615	db7950fc515f	37eb1ba4ae55	Blitz Rakete	Added a warning when no plugins could be loaded, see T2114 for more details. (Show More…)	Dec 1 2016, 7:33 PM

Diff 8690

View Options

src/lib/marble/PluginManager.cpp