Fix Qt 5.5 compatibility -- no constFirst/Last methods.
ClosedPublic

Authored by flherne on Oct 8 2017, 3:27 PM.

Details

Summary

QList and QVector both gained these in Qt 5.6, but we claim to support 5.5 (because it's found in Ubuntu 16.04 LTS).

None of these are in performance-relevant code, so I don't believe it's worthwhile adding ifdefs; just revert this patch whenever 5.5 support is dropped.

Reported by 'blaze' on IRC.

Diff Detail

Repository
R32 KDevelop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
flherne created this revision.Oct 8 2017, 3:27 PM
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptOct 8 2017, 3:27 PM

None of these are in performance-relevant code, so I don't believe it's worthwhile adding ifdefs; just revert this patch whenever 5.5 support is dropped.

Possible problems: the person bumping min qt version to > 5.5 will not remember about this. And the same for the next person "fixing" this non-const calls.

So while ifdef adds some another code variant which makes test coverage worse, it still help cleaning up the code on version bumps and prevents compat-breaking "fixing". So I would opt for ifdef.

mwolff requested changes to this revision.Oct 8 2017, 7:02 PM
mwolff added a subscriber: mwolff.

I'd actually say let's do the opposite: Let's require 5.6 and drop direct support for the older Ubuntu system. If people want to use a newer KDevelop there, they can simply use the AppImage and be done with it.

I also bet that there are backport repos for 16.04 they could use. There's a 5.9 backport repo for 14.04 after all...

Kevin, Sven, what do you think?

This revision now requires changes to proceed.Oct 8 2017, 7:02 PM
kfunk added a subscriber: kfunk.Oct 8 2017, 7:09 PM

My suggestion:
<kfunk> milian: re. https://phabricator.kde.org/D8202 okay to let 5.2 require Qt 5.5 (merge this patch), and make master require Qt 5.6 (revert it again here)?
<kfunk> so I can tell 16.04 LTS users to use 5.2 as a fallback
<milian> kfunk: sure, this was just my suggestion

blaze added a subscriber: blaze.Oct 8 2017, 7:10 PM

None of these are in performance-relevant code, so I don't believe it's worthwhile adding ifdefs; just revert this patch whenever 5.5 support is dropped.

Possible problems: the person bumping min qt version to > 5.5 will not remember about this. And the same for the next person "fixing" this non-const calls.

So while ifdef adds some another code variant which makes test coverage worse, it still help cleaning up the code on version bumps and prevents compat-breaking "fixing". So I would opt for ifdef.

First issue can be solved by adding a task to a job list for the certain milestone where Qt5.5 support should be dropped, not sure if such list even exists but still. Second is more simple: just comment the code that needs special attention. Not a rock-solid solution but still a possibility of having cleaner code yet being able to manage this stuff at some point in the future.

This revision was automatically updated to reflect the committed changes.