Make use of KPluginMetaData where we can
ClosedPublic

Authored by apol on Sep 2 2019, 5:11 PM.

Details

Summary

Instead of relying on the older KPluginInfo which is slower (parses de metadata twice) and comes from a much higher tier.

Test Plan

Running plasmashell with it

Diff Detail

Repository
R242 Plasma Framework (Library)
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
apol created this revision.Sep 2 2019, 5:11 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptSep 2 2019, 5:11 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
apol requested review of this revision.Sep 2 2019, 5:11 PM
broulik added a subscriber: broulik.Sep 2 2019, 6:09 PM
broulik added inline comments.
src/plasma/containmentactions.h
71–73
@deprecated since 5.62 use metadata
src/plasma/dataengine.cpp
70

Where is this constructor used?

src/plasma/dataengine.h
82

Docs, with @since

124

5

src/plasma/scripting/dataenginescript.cpp
91–96

Does this need a KPluginMetadata getter?

src/plasma/theme.cpp
444

Does this need a separate metadata() getter?

src/plasma/theme.h
323–325

Separate @deprecated since line

apol updated this revision to Diff 65327.Sep 3 2019, 2:31 PM

address comments

apol updated this revision to Diff 65328.Sep 3 2019, 2:37 PM
apol marked 6 inline comments as done.

Forgot these

apol added inline comments.Oct 1 2019, 1:15 AM
src/plasma/dataengine.cpp
70

Unsure, but we can't remove symbols anyway...

apol updated this revision to Diff 68544.Oct 22 2019, 1:48 PM

Adopt new deprecation macros

apol updated this revision to Diff 68546.Oct 22 2019, 1:51 PM

Unrelated change

mart accepted this revision.Dec 11 2019, 2:15 PM
This revision is now accepted and ready to land.Dec 11 2019, 2:15 PM

Do not forget to bump 5.64 to 5.66 in all places :)

Also some nitpicks on the style... remember that API dox tool and compiler both need all data duplicated sadly, both deprecated-at version and what-to-do-instead.

Please also compare https://community.kde.org/Policies/Library_Code_Policy#Deprecation_of_API and improve where needed,

src/plasma/containmentactions.h
73

Also add Since note to deprecation, for people reading the generated API dox.
So:

@deprecated Since 5.66, use metadata()
75

Also recommended: put PLASMA_DEPRECATED_VERSION onto own line, makes parsing the code more easy

PLASMA_DEPRECATED_VERSION(5, 64, "use metadata()")
KPluginInfo pluginInfo() const;
src/plasma/dataengine.h
77

For people reading the generated API dox, we need to duplicate the recommendation what to do:

@deprecated Since 5.66, use xyz
80–91

own line again recommended

119

own line

src/plasma/scripting/dataenginescript.h
114

No visibility wrappers here? (#if PLASMA_ENABLE_DEPRECATED_SINCE(5, 66))

117

@deprecated Since 5.66, use metadata()

src/plasma/theme.h
327

own line

apol marked 8 inline comments as done.Jan 12 2020, 1:38 AM
apol updated this revision to Diff 73322.Jan 12 2020, 1:38 AM

Addressed kossebau's comments

This revision was automatically updated to reflect the committed changes.
kossebau added inline comments.Jan 12 2020, 6:26 AM
src/plasma/containmentactions.h
67

The visibility wrappers also need bumping to 5, 67, so the version were the API got deprecated, to not break things for people who use *_DISABLE_DEPRECATED_BEFORE_AND_AT with 5.64-5.66 and who still use this API.

Will fix directly now. just mentioning here as reminder for future similar deprecation patches.

And yes, this data duplication sucks, but no-one found a way around yet for this macro magic to have the features of control both about visibility and warnings separately.