D28232: [SpellChecking KCM] Fix the build

[Ahmad Samir]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:646bd3eecc72: [SpellChecking KCM] Fix the build (authored 
by ahmadsamir).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28232?vs=78554=78555

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

AFFECTED FILES
  CMakeLists.txt
  kcms/spellchecking/spellchecking.cpp

To: ahmadsamir, #plasma, bport, meven, crossi, ervin, davidedmundson
Cc: davidedmundson, bcooksley, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D28232: [SpellChecking KCM] Fix the build

[Ahmad Samir]

ahmadsamir updated this revision to Diff 78554.
ahmadsamir added a comment.


  Rebase

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28232?vs=78347=78554

BRANCH
  l-fix-build (branched from master)

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

AFFECTED FILES
  CMakeLists.txt
  kcms/spellchecking/spellchecking.cpp

To: ahmadsamir, #plasma, bport, meven, crossi, ervin, davidedmundson
Cc: davidedmundson, bcooksley, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D28232: [SpellChecking KCM] Fix the build

[David Edmundson]

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


  > 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.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  l-fix-build (branched from master)

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

To: ahmadsamir, #plasma, bport, meven, crossi, ervin, davidedmundson
Cc: davidedmundson, bcooksley, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D28232: [SpellChecking KCM] Fix the build

[Ben Cooksley]

bcooksley added a comment.


  @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.

REPOSITORY
  R119 Plasma Desktop

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

To: ahmadsamir, #plasma, bport, meven, crossi, ervin
Cc: bcooksley, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28232: [SpellChecking KCM] Fix the build

[Ahmad Samir]

ahmadsamir added a comment.


  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.

REPOSITORY
  R119 Plasma Desktop

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

To: ahmadsamir, #plasma, bport, meven, crossi, ervin
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28232: [SpellChecking KCM] Fix the build

[Benjamin Port]

bport added a comment.


  https://community.kde.org/Schedules/Plasma_5#Support_status_by_Release_Series
  So we need to see if we can update the CI

REPOSITORY
  R119 Plasma Desktop

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

To: ahmadsamir, #plasma, bport, meven, crossi, ervin
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28232: [SpellChecking KCM] Fix the build

[Ahmad Samir]

ahmadsamir added a comment.


  In D28232#634281 , @bport wrote:
  
  > 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.

REPOSITORY
  R119 Plasma Desktop

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

To: ahmadsamir, #plasma, bport, meven, crossi, ervin
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28232: [SpellChecking KCM] Fix the build

[Benjamin Port]

bport added a comment.


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

INLINE COMMENTS

> CMakeLists.txt:9
>  set(QT_MIN_VERSION "5.12.0")
> -set(KF5_MIN_VERSION "5.66.0")
>  

Indeed this is needed thanks

REPOSITORY
  R119 Plasma Desktop

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

To: ahmadsamir, #plasma, bport, meven, crossi, ervin
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28232: [SpellChecking KCM] Fix the build

[Ahmad Samir]

ahmadsamir created this revision.
ahmadsamir added reviewers: Plasma, bport, meven, crossi, ervin.
Herald added a project: Plasma.
ahmadsamir requested review of this revision.

REVISION 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

REPOSITORY
  R119 Plasma Desktop

BRANCH
  l-fix-build (branched from master)

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

AFFECTED FILES
  CMakeLists.txt
  kcms/spellchecking/spellchecking.cpp

To: ahmadsamir, #plasma, bport, meven, crossi, ervin
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart