D4883: Add IProblem::setPluginName() method

Milian Wolff noreply at phabricator.kde.org
Sun Mar 5 19:32:12 UTC 2017


mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.


  As-is, this is not a good approach imo. Either have it as generic API in iproblem.h (which would mean you'd also need to serialize the data in problem.cpp), or have it as a special case for the detected problem and only use it there

INLINE COMMENTS

> iproblem.h:96
> +    ///
> +    /// The method call only makes sense when the problem source is IProblem::Plugin.
> +    virtual void setPluginName(const QString& name) = 0;

I don't understand this part, can you elaborate please?

also, I'd say this should be setSourceString then and the default implementation in `problem.h` should do something

as-is, this API is highly confusing

> problem.cpp:216
>  {
>      switch (source()) {
>          case IProblem::Disk:

this should check if the source has been set explicitly, and if so, return that directly

> problem.cpp:238
> +void Problem::setPluginName(const QString& name)
> +{
> +}

this should do something

> problem.h:149
>      QString sourceString() const override;
> +    void setPluginName(const QString& name) override;
>  

add APIDOX saying that the default implementation does nothing

REPOSITORY
  R33 KDevPlatform

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

To: antonanikin, #kdevelop, mwolff
Cc: mwolff, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170305/c78d7b73/attachment-0001.html>


More information about the KDevelop-devel mailing list