Compiz

Merge lp://qastaging/~vanvugt/compiz/remove-availablePlugins into lp://qastaging/compiz/0.9.8

remove-availablePlugins
Merge into 0.9.8

Proposed by Daniel van Vugt on 2012-06-10

Status:	Merged
Merged at revision:	3247
Proposed branch:	lp://qastaging/~vanvugt/compiz/remove-availablePlugins
Merge into:	lp://qastaging/compiz/0.9.8
Diff against target:	648 lines (+7/-382) 8 files modified include/core/plugin.h (+0/-6) plugins/dbus/src/dbus.cpp (+7/-55) plugins/dbus/src/dbus.h (+0/-4) src/plugin.cpp (+0/-79) src/plugin/tests/CMakeLists.txt (+0/-18) src/plugin/tests/test-loader.cpp (+0/-72) src/plugin/tests/test-plugin.cpp (+0/-103) src/privatescreen/tests/test-privatescreen.cpp (+0/-45)
To merge this branch:	bzr merge lp://qastaging/~vanvugt/compiz/remove-availablePlugins
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Sam Spilsbury		2012-06-10	Approve on 2012-06-10
Review via email: mp+109521@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2012-06-08.

Description of the change

Remove dead code: availablePlugins() and *ListPlugins()

availablePlugins is unused except by dbus, and flawed by designed. It makes
no sense to have a function that claims to return the list of available
plugins, when that list is not complete. You could easily load other plugins
from LD_LIBRARY_PATH that availablePlugins doesn't know about. And you could
add or remove plugin binaries at runtime which would also invalidate what
availablePlugins knows. ListPlugins was only used by availablePlugins.

If you want a list of known plugins, call CompPlugin::getPlugins() instead.

Revision history for this message

Sam Spilsbury (smspillaz) wrote on 2012-06-08: Posted in a previous version of this proposal

Hi, 2 things.

1. Remove the if block around the local plugins member in dbus (or reimplement getplugins in terms if compplugins::getplugins).
2. Rework the tests around getPlugins if they arent there

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2012-06-10:

Done #1. Skipped #2 because TEST(PluginTest, list_plugins_*) tests for functionality that no longer exists and was dead code anyway.

The only exception is:
TEST(PluginTest, list_plugins_are_deduped)
however there are still tests for de-duplication here:
TEST(privatescreen_PluginManagerTest, verify_plugin_ordering)