search

tibirna / qgit

Official git repository for QGit.
Other
174 stars 68 forks source link

Address compiler warnings about deprecated QString::SplitBehavior for compiling with Qt >=5.14 #114

Closed hartwork closed 3 years ago

hartwork commented 3 years ago

Replace QString::SplitBehavior by Qt::SplitBehavior of Qt >=5.14

Docs:

Addresses this compiler warning:

warning: ‘QStringList QString::split(QChar, QString::SplitBehavior, Qt::CaseSensitivity) const’ is deprecated: Use Qt::SplitBehavior variant instead [-Wdeprecated-declarations]
hartwork commented 3 years ago

@tibirna turns out Qt >=5.14 would need Ubuntu >=20.10 (which can be made to work, if desired). What is the oldest version of Qt that you want to require to be supported by QGit? I'm asking both for this very pull request as well as in general. Thanks!

Note to self:

tibirna commented 3 years ago

My will doesn't have much to say in this. We need to ensure that the code remains directly compilable on the popular Linux distros (I guess for Windows and MacOS a compilation setup is sufficiently custom anyways, so there won't be hindrances there). I see that the first Qt5 LTS is Qt-5.12, from May 2021, which is unfortunate, as it's too short a run for most Linux distros to settle on it. Debian seems to use 5.11 still.

So, the logical conclusion is, QGit has to compile with Qt-5.11 for now.

hartwork commented 3 years ago

My will doesn't have much to say in this. We need to ensure that the code remains directly compilable on the popular Linux distros (I guess for Windows and MacOS a compilation setup is sufficiently custom anyways, so there won't be hindrances there). I see that the first Qt5 LTS is Qt-5.12, from May 2021, which is unfortunate, as it's too short a run for most Linux distros to settle on it. Debian seems to use 5.11 still.

So, the logical conclusion is, QGit has to compile with Qt-5.11 for now.

@tibirna thanks for elaborating. Please note that the 5.11 in Debian is oldstable (buster) not stable (bullseye). I had a look myself now:

Ubuntu 20.04-LTS/focal   5.12.8
Ubuntu 20.10/groovy      5.14.2
Debian oldstable/buster  5.11.3
Debian stable/bullseyes  5.15.2
Gentoo                   5.15.2

All other warnings I could fix with Qt 5.11 (new pull request #115) so I have no reason to campaign for 5.12 right now :smiley: Looking forward to your review over there.

a17r commented 3 years ago

I see that the first Qt5 LTS is Qt-5.12, from May 2021, which is unfortunate, as it's too short a run for most Linux distros to settle on it. Debian seems to use 5.11 still.

That's incorrect. It may be the latest LTS 5.12 point release, but 5.12.0 was released on 2018-12-06.

hartwork commented 3 years ago

@tibirna I added a prefix "[Qt >=5.14]" to this pull request's title now to make its effect and requirements easy to see in the future. That is if we'd like to have the PR sit here and wait for potential merging in the future. Closing it as "too early" would also work for me, no hard feelings about it. What do you think?

tibirna commented 3 years ago

Having PRs sit around for long is a bit cumbersome. There is a potential for accumulating them to a point where they become difficult to follow. Dropping a bit of already done work as "too early" is also a waste. I think the better solution would be to use version guards (preprocessor instructions) to conditionally apply the changes depending on the library version used in the compilation. I know you tried hard to eliminate such constructs related to Qt4, but, for the better or worse, such tools are useful when dealing with libraries introducing incompatible changes in successive version.

hartwork commented 3 years ago

@tibirna alright, that's something I can try. I guess it would be cleaner to have that version ifdef at a single place for SkipEmptyParts and KeepEmptyParts rather than spreading it all over the place. I can try to find a clean way to do that. Maybe let's do that after #115 has been merged. Good?

hartwork commented 3 years ago

Only saw now that #115 has just been merged. I can start reshaping #114 already then, good.

hartwork commented 3 years ago

I think I'll put a new macro into common.hfor a start. If you'd rather have me start a new file compat.h or so, let me know.

hartwork commented 3 years ago

Also, I'll put things in a new commit for now, happy to squash-rebase things later as desired.

hartwork commented 3 years ago

@tibirna here's one way how it could be done, please let me know what you'd want me to adjust.