[SpellChecking KCM] Fix the build
ClosedPublic

Authored by ahmadsamir on Mar 24 2020, 10:56 AM.

Details

Summary

QList/QSet iterator-based ctors are available since Qt 5.14, so we
could make the code conditional based on that to keep it building
against older Qt versions. However I think the reason behind converting
from QStringList to QSet was to remove duplicates, so use
QStringList::removeDuplicates() and sort() instead.

Bump KF5 min. required version to 5.69.0 because of Sonnet/ConfigView
which first appeared in that version.

Test Plan

make && ctest

Diff Detail

Repository
R119 Plasma Desktop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ahmadsamir created this revision.Mar 24 2020, 10:56 AM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 24 2020, 10:56 AM
ahmadsamir requested review of this revision.Mar 24 2020, 10:56 AM

Next version of plasma will depend on Qt 5.14 AFAIK so it's not a problem to depend on it

CMakeLists.txt
9

Indeed this is needed thanks

Next version of plasma will depend on Qt 5.14 AFAIK so it's not a problem to depend on it

But the CI still uses 5.12 for the opensuse image, and probably 5.13 for the freebsd AFAIK.

I don't know when the CI will be updated, probably soon based on what I read on IRC #kde-devel, but until then isn't it better to keep the CI working for plasma-desktop? I could rework the patch to make it conditional on 5.14 and keep the iterator-based ctors as-is.

@bport Sorry, but this cannot be left in a broken state like this.

Reality is that the plasma-desktop repository declares that it depends on a minimum of Qt 5.12 at the moment in CMake, so that is what it needs to build with.
Should this be insufficient, then that needs to be corrected.

You cannot just bump that dependency though, as notice needs to be given to the CI system maintainers at a minimum of 2 weeks in advance of such a change.
To date, no notice has been received from Plasma of this.

Discussion of a change on any mailing list, regardless of whether the CI maintainers are subscribed to the list or not does not constitute giving this notice - you need to open an issue on build.kde.org or file a Sysadmin ticket to give this notice.

davidedmundson accepted this revision.Mar 26 2020, 2:38 PM
davidedmundson added a subscriber: davidedmundson.

I could rework the patch to make it conditional on 5.14 and keep the iterator-based ctors as-is.

May as well merge this and just revert when we're able to use 5.14

The alternative is more work.

This revision is now accepted and ready to land.Mar 26 2020, 2:38 PM
This revision was automatically updated to reflect the committed changes.