The recent changes to https://invent.kde.org/graphics/digikam/-/tree/gsoc21-qt6-port makes it compile only on newer Qt versions. So I it should be best Bump Qt version to 5.15.2. Should I make the change? Thanks, Anjani
|
Hi, No. We need to be compatible at least with Qt 5.12 LTS. Please make pre-processeur branching in source code by checking Qt version, as it already do in other place in digiKam. Best Gilles Caulier Le mar. 15 juin 2021 à 13:52, Anjani Kumar <[hidden email]> a écrit :
|
But we already have a problem with Qt::endl, this only exists with Qt-5.14. We
should find another way of doing this than with pre-processeur branching. Maybe define a macro. Maik Am Dienstag, 15. Juni 2021, 13:55:53 CEST schrieb Gilles Caulier: > Hi, > > No. We need to be compatible at least with Qt 5.12 LTS. > Please make pre-processeur branching in source code by checking Qt version, > as it already do in other place in digiKam. > > exemple : > https://invent.kde.org/graphics/digikam/-/blob/master/core/app/date/ddatepic > ker.cpp#L547 > > Best > > Gilles Caulier > > Le mar. 15 juin 2021 à 13:52, Anjani Kumar <[hidden email]> a écrit : > > The recent changes to > > https://invent.kde.org/graphics/digikam/-/tree/gsoc21-qt6-port makes it > > compile only on newer Qt versions. So I it should be best Bump Qt version > > to 5.15.2. Should I make the change? > > > > Thanks, > > Anjani |
I already started this pre-processor branching. Not for Qt::endl but I have done it for QString::split() which is also present from Qt 5.14. On Jun 16 2021, at 2:16 am, Maik Qualmann <[hidden email]> wrote:
|
Something like this
I haven't pushed it yet though. On Jun 16 2021, at 2:19 am, Anjani Kumar <[hidden email]> wrote:
|
What should I do? Keep doing this or not? Thanks Anjani On Jun 16 2021, at 2:23 am, Anjani Kumar <[hidden email]> wrote:
|
It's all correct with the Qt version check. My only thought was whether we
could find another option for Qt::endl, as there would be a lot of Qt version checks. Maik Am Dienstag, 15. Juni 2021, 23:09:28 CEST schrieb Anjani Kumar: > What should I do? Keep doing this or not? > > Thanks > Anjani > > On Jun 16 2021, at 2:23 am, Anjani Kumar <[hidden email]> wrote: > > Something like this > > > > > > #if (QT_VERSION >= QT_VERSION_CHECK(5, 14, 0)) > > > > QStringList tagsList = hierarchy.split(QLatin1Char('/'), > > Qt::SkipEmptyParts); > > > > #else > > > > QStringList tagsList = hierarchy.split(QLatin1Char('/'), > > QString::SkipEmptyParts); > > > > #endif > > I haven't pushed it yet though. > > > > On Jun 16 2021, at 2:19 am, Anjani Kumar <[hidden email]> wrote: > > > I already started this pre-processor branching. Not for Qt::endl but I > > > have done it for QString::split() which is also present from Qt 5.14.> > > > > On Jun 16 2021, at 2:16 am, Maik Qualmann <[hidden email]> wrote: > > > > But we already have a problem with Qt::endl, this only exists with > > > > Qt-5.14. We should find another way of doing this than with > > > > pre-processeur branching. Maybe define a macro. > > > > > > > > Maik > > > > > > > > Am Dienstag, 15. Juni 2021, 13:55:53 CEST schrieb Gilles Caulier: > > > > > Hi, > > > > > > > > > > No. We need to be compatible at least with Qt 5.12 LTS. > > > > > Please make pre-processeur branching in source code by checking Qt > > > > > version, > > > > > as it already do in other place in digiKam. > > > > > > > > > > exemple : > > > > > https://invent.kde.org/graphics/digikam/-/blob/master/core/app/date/ > > > > > ddatepic ker.cpp#L547 > > > > > > > > > > Best > > > > > > > > > > Gilles Caulier > > > > > > > > > > Le mar. 15 juin 2021 à 13:52, Anjani Kumar <[hidden email]> a > > > > > > The recent changes to > > > > > > https://invent.kde.org/graphics/digikam/-/tree/gsoc21-qt6-port > > > > > > makes it > > > > > > compile only on newer Qt versions. So I it should be best Bump Qt > > > > > > version > > > > > > to 5.15.2. Should I make the change? > > > > > > > > > > > > Thanks, > > > > > > Anjani |
Hi, Using digikam_globasl.h with a preprocessor definition as QT_ENDL depending of Qt version can be a solution. Gilles Le mer. 16 juin 2021 à 07:41, Maik Qualmann <[hidden email]> a écrit : It's all correct with the Qt version check. My only thought was whether we |
Hello, This should be fine I guess, https://invent.kde.org/graphics/digikam/-/commit/0a1fd2c60ec38707f0e2dfeabf6ebb45a01f9382. Let me know. Thanks, Anjani On Jun 16 2021, at 11:23 am, Gilles Caulier <[hidden email]> wrote:
|
Hello, I defined the macro QT_ENDL appropriately, and made the changes locally. Should I push them? Thanks, Anjani On Jun 16 2021, at 12:35 pm, Anjani Kumar <[hidden email]> wrote:
|
If everything compiles fine, yes. I think it would be a good solution for
QString::SkipEmptyParts as well. Maik Am Donnerstag, 17. Juni 2021, 09:23:31 CEST schrieb Anjani Kumar: > Hello, > I defined the macro QT_ENDL appropriately, and made the changes locally. > Should I push them? > > Thanks, > Anjani > > On Jun 16 2021, at 12:35 pm, Anjani Kumar <[hidden email]> wrote: > > Hello, > > > > This should be fine I guess, > > https://invent.kde.org/graphics/digikam/-/commit/0a1fd2c60ec38707f0e2dfea > > bf6ebb45a01f9382. Let me know. Thanks, > > Anjani > > > > On Jun 16 2021, at 11:23 am, Gilles Caulier <[hidden email]> > > > Hi, > > > > > > Using digikam_globasl.h with a preprocessor definition as QT_ENDL > > > depending of Qt version can be a solution. > > > > > > Gilles > > > > > > Le mer. 16 juin 2021 à 07:41, Maik Qualmann <[hidden email] (mailto:[hidden email])> a écrit : > > > > It's all correct with the Qt version check. My only thought was > > > > whether we > > > > could find another option for Qt::endl, as there would be a lot of Qt > > > > version checks. > > > > > > > > Maik > > > > > > > > Am Dienstag, 15. Juni 2021, 23:09:28 CEST schrieb Anjani Kumar: > > > > > What should I do? Keep doing this or not? > > > > > > > > > > Thanks > > > > > Anjani > > > > > > > > > > On Jun 16 2021, at 2:23 am, Anjani Kumar <[hidden email] > > > > > > Something like this > > > > > > > > > > > > > > > > > > #if (QT_VERSION >= QT_VERSION_CHECK(5, 14, 0)) > > > > > > > > > > > > QStringList tagsList = hierarchy.split(QLatin1Char('/'), > > > > > > Qt::SkipEmptyParts); > > > > > > > > > > > > #else > > > > > > > > > > > > QStringList tagsList = hierarchy.split(QLatin1Char('/'), > > > > > > QString::SkipEmptyParts); > > > > > > > > > > > > #endif > > > > > > I haven't pushed it yet though. > > > > > > > > > > > > On Jun 16 2021, at 2:19 am, Anjani Kumar <[hidden email] > > > > > > > I already started this pre-processor branching. Not for Qt::endl > > > > > > > but I > > > > > > > have done it for QString::split() which is also present from Qt > > > > > > > 5.14.> > > > > > > > > > > > > > > > On Jun 16 2021, at 2:16 am, Maik Qualmann <[hidden email] (mailto:[hidden email])> wrote: > > > > > > > > But we already have a problem with Qt::endl, this only exists > > > > > > > > with > > > > > > > > Qt-5.14. We should find another way of doing this than with > > > > > > > > pre-processeur branching. Maybe define a macro. > > > > > > > > > > > > > > > > Maik > > > > > > > > > > > > > > > > Am Dienstag, 15. Juni 2021, 13:55:53 CEST schrieb Gilles Caulier: > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > No. We need to be compatible at least with Qt 5.12 LTS. > > > > > > > > > Please make pre-processeur branching in source code by > > > > > > > > > checking Qt > > > > > > > > > version, > > > > > > > > > as it already do in other place in digiKam. > > > > > > > > > > > > > > > > > > exemple : > > > > > > > > > https://invent.kde.org/graphics/digikam/-/blob/master/core/a > > > > > > > > > pp/date/ > > > > > > > > > ddatepic ker.cpp#L547 > > > > > > > > > > > > > > > > > > Best > > > > > > > > > > > > > > > > > > Gilles Caulier > > > > > > > > > > > > > > > > > > Le mar. 15 juin 2021 à 13:52, Anjani Kumar > > > > > > > > > <[hidden email] (mailto:[hidden email])> a> > > > > > > écrit : > > > > > > > > > > The recent changes to > > > > > > > > > > https://invent.kde.org/graphics/digikam/-/tree/gsoc21-qt6-> > > > > > > > > > port > > > > > > > > > > makes it > > > > > > > > > > compile only on newer Qt versions. So I it should be best > > > > > > > > > > Bump Qt > > > > > > > > > > version > > > > > > > > > > to 5.15.2. Should I make the change? > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > Anjani |
Yes it would. I did realize it after making commits otherwise. https://invent.kde.org/graphics/digikam/-/commit/8b651e62b3133908aa9500e1f7a984ec0de13531 :( Should I correct it? Thanks Anjani On Jun 17 2021, at 9:39 pm, Maik Qualmann <[hidden email]> wrote:
|
I cleared all the issues above Qt 5.12 including macros for Qt::endl and Qt::SplitBehavior. Compilations is working fine on Qt 5.15.2 but I have no way to compile it on Qt 5.12.0 as of now. Also, I have one MR https://invent.kde.org/graphics/digikam/-/merge_requests/95. Could you review it? I have fixed some of the concerns. Thanks, Anjani On Jun 17 2021, at 9:41 pm, Anjani Kumar <[hidden email]> wrote:
|
Hi Anjani, Please review the new cppcheck warnings here : cppcheck has now a specific Qt config file which reports more warnings about deprecated API. Typically, i expect that qrandCalled, useStlAlgorithm, qsrandCalled, and qSortCalled entries will be fixed in your qt6 port project Right ? Best Gilles Caulier Le ven. 18 juin 2021 à 11:51, Anjani Kumar <[hidden email]> a écrit :
|
Anjani, Did you sync your Qt§ branch with master reguliary ? It's important as master code continue to live a lots... Best Gilles Caulier Le ven. 18 juin 2021 à 13:27, Gilles Caulier <[hidden email]> a écrit :
|
Yes I do rebase regularly. As suggested by Maik, I do it on the weekends or complete a task Thanks Anjani On Jun 18 2021, at 4:59 pm, Gilles Caulier <[hidden email]> wrote:
|
In reply to this post by Gilles Caulier-4
For qrand() and qsrand() I had already proposed them to replace with QRandomGenerator in the proposal and I plan to do it next week alongwith few other fixes. So it should be no problem. The other issues also seem straight forward as I have looked. So it shouldn't be a problem. Also How can I run these checks locally? Thanks Anjani On Jun 18 2021, at 4:57 pm, Gilles Caulier <[hidden email]> wrote:
|
yes, this can be run locally, but : 1/ sync your branch (i just add a new option to not update web site page with static analyzer results) 2/ go to project/reports/ 3/ fix cppcheck dependencies (look on top of ./cppcheck.sh script) 4/ run ./cppcheck.sh --nowebupdate. This will parse the whole digiKam source code. This can take a few minutes. 5/ local results are in html page located in ./report.cppcheck Gilles Caulier |
Thanks , will do Anjani On Jun 18 2021, at 5:31 pm, Gilles Caulier <[hidden email]> wrote:
|
How should I push STL changes for review? I am thinking of opening a Work in progress MR and keep committing changes in it and you can continue reviewing the MR until it is finished and then merge? Thanks Anjani On Jun 18 2021, at 6:39 pm, Anjani Kumar <[hidden email]> wrote:
|
Free forum by Nabble | Edit this page |