Deprecated allowAsDefault
ClosedPublic

Authored by nicolasfella on Dec 7 2019, 12:09 AM.

Details

Summary

Execute upon T12309. The KServiceOffer still takes a 'bool allowedAsDefault', that needs an overload, but in a different patch

Test Plan

builds

Diff Detail

Repository
R309 KService
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
nicolasfella created this revision.Dec 7 2019, 12:09 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptDec 7 2019, 12:09 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
nicolasfella requested review of this revision.Dec 7 2019, 12:09 AM
kossebau added inline comments.
src/services/kservice.cpp
982

This should be BUILD, not ENABLE.

*BUILD* macros are controlled by the EXCLUDE_DEPRECATED_BEFORE_AND_AT value, which iis what is used at build time of the library itself, and then hardcoded with the installed headers.
*ENABLE* macros are controlled by KF_DISABLE_DEPRECATED_BEFORE_AND_AT value, which is used when building against the library, and can be controlled by the user of the library.
To avoid duplication, the *ENABLE* macros are also reused in the headers during the build time of the library itself, to not to have to write

#if KSERVICE_ENABLE_DEPRECATED_SINCE(5, 65) AND KSERVICE_BUILD_DEPRECATED_SINCE(5, 65)

While it works to use *ENABLE* also in the sources, like the reuse works in the headers, it is bad practice as it blurs the purposes of the ENABLE vs the BUILD macros, where the latter is only to be set at build time of the library itself. So to not give people wrong ideas, the BUILD macros should be used everywhere where only in the build of the library itself it is deciced via the EXCLUDE_DEPRECATED_BEFORE_AND_AT value whether code should be part of the created library.

src/services/kserviceoffer.cpp
98

KSERVICE_BUILD_DEPRECATED_SINCE

  • Fix and update
dfaure accepted this revision.Sun, Jan 19, 10:06 PM
This revision is now accepted and ready to land.Sun, Jan 19, 10:06 PM
This revision was automatically updated to reflect the committed changes.

This makes kde-cli-tools fail to build for me with Qt 5.14 on openSUSE Tumbleweed:

[ 43%] Linking CXX executable ../bin/kioclient5
/home/nate/kde/src/kde-cli-tools/keditfiletype/mimetypedata.cpp: In member function ‘QStringList MimeTypeData::getAppOffers() const’:
/home/nate/kde/src/kde-cli-tools/keditfiletype/mimetypedata.cpp:217:20: error: ‘class KService’ has no member named ‘allowAsDefault’
  217 |         if ((*it)->allowAsDefault())
      |                    ^~~~~~~~~~~~~~

I guess we found a use case. :)

Removed: https://commits.kde.org/kde-cli-tools/e435a827bba9ebfbe7f988fe063b5848f5ff524b
(also merged to master)

It's not an actual use case for the feature when zero desktop file sets this field :)