D8156: Fix registration of the enabled state of plugins

Milian Wolff noreply at phabricator.kde.org
Mon Oct 16 18:03:33 UTC 2017


mwolff accepted this revision.
mwolff added a comment.
This revision is now accepted and ready to land.


  minor nits, otherwise lgtm

INLINE COMMENTS

> plugincontroller.cpp:748
>          }
> +        // TODO: what about project plugins? what about dependency plugins?
>      }

yes, what about them? Sorry, but it's been a long time since I worked on this code. Can you elaborate the problems you are seeing?

> globaldefaultplugin.cpp:30
>  public:
> -    virtual ~ITestNonGuiInterface() {}
> +    GlobalDefaultPlugin(QObject* parent, const QVariantList&);
>  };

here and below, make ctors explicit

> nonguiinterfaceplugin.cpp:33
> +public:
> +    NonGuiInterfacePlugin(QObject* parent, const QVariantList&);
> +};

explicit

> projectdefaultplugin.cpp:30
>  public:
> -    virtual ~ITestNonGuiInterface() {}
> +    ProjectDefaultPlugin(QObject* parent, const QVariantList&);
>  };

explicit

> projectnondefaultplugin.cpp:30
>  public:
> -    virtual ~ITestNonGuiInterface() {}
> +    ProjectNonDefaultPlugin(QObject* parent, const QVariantList&);
>  };

explicit

> test_pluginenabling.cpp:100
> +    });
> +    // TODO: somehow currently the clean-up of the previous session is not yet done
> +    // on the next data item test run, so the session with the name is still locked

I'd have to debug that myself, it should work... maybe add a signal spy on some signal to ensure the core gets destroyed or otherwise shutdown properly? I.e. maybe we need to run the event loop?

> test_pluginenabling.cpp:103
> +    // so we work-around that for now by using a custom session name per session
> +    // TODO: consider adding a new bool temporarySession = true to TestCore::initialize()
> +    TestCore::initialize(Core::NoUi, QStringLiteral("test_pluginenabling_custom_")+pluginId);

sure, why not. So far, the `empty name == temporary` rule worked fine afaik. If there's more needed, extend the API as needed.

REPOSITORY
  R32 KDevelop

BRANCH
  fixEstimatingDefaultEnabledPlugin_v1

REVISION DETAIL
  https://phabricator.kde.org/D8156

To: kossebau, #kdevelop, mwolff
Cc: mwolff, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20171016/6ef8a64b/attachment.html>


More information about the KDevelop-devel mailing list