Review Request 122556: Bump Qt version to 5.4

[Arjun AK]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122556/
---

Review request for KDE Base Apps.


Repository: kde-baseapps


Description
---

`QUrl::fromUserInput(userInput, workingDirectory, UserInputResolutionOptions 
options)` seems to be Qt 5.4+. We either need to bump requirement to 5.4 or do 
what kompare 
[did](https://projects.kde.org/projects/kde/kdesdk/kompare/repository/revisions/master/entry/libdialogpages/diffpage.cpp#L45)


Diffs
-

  CMakeLists.txt 6df9015 

Diff: https://git.reviewboard.kde.org/r/122556/diff/


Testing
---


Thanks,

Arjun AK

Re: Review Request 122556: Bump Qt version to 5.4

[Martin Gräßlin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122556/#review75971
---


AFAIK our CI system doesn't support Qt 5.4 yet. I think we should first ensure 
the CI system is prepared for building with the new dependency.

- Martin Gräßlin


On Feb. 13, 2015, 9:06 a.m., Arjun AK wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122556/
> ---
> 
> (Updated Feb. 13, 2015, 9:06 a.m.)
> 
> 
> Review request for KDE Base Apps.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> ---
> 
> `QUrl::fromUserInput(userInput, workingDirectory, UserInputResolutionOptions 
> options)` seems to be Qt 5.4+. We either need to bump requirement to 5.4 or 
> do what kompare 
> [did](https://projects.kde.org/projects/kde/kdesdk/kompare/repository/revisions/master/entry/libdialogpages/diffpage.cpp#L45)
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 6df9015 
> 
> Diff: https://git.reviewboard.kde.org/r/122556/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun AK
> 
>

Re: Review Request 122556: Bump Qt version to 5.4

[Albert Astals Cid]

> On feb. 13, 2015, 8:41 a.m., Martin Gräßlin wrote:
> > AFAIK our CI system doesn't support Qt 5.4 yet. I think we should first 
> > ensure the CI system is prepared for building with the new dependency.

It does
http://build.kde.org/job/kde-baseapps_master_qt5/238/consoleFull 
 qt5 - Branch 5.4.1


- Albert


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122556/#review75971
---


On feb. 13, 2015, 8:06 a.m., Arjun AK wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122556/
> ---
> 
> (Updated feb. 13, 2015, 8:06 a.m.)
> 
> 
> Review request for KDE Base Apps.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> ---
> 
> `QUrl::fromUserInput(userInput, workingDirectory, UserInputResolutionOptions 
> options)` seems to be Qt 5.4+. We either need to bump requirement to 5.4 or 
> do what kompare 
> [did](https://projects.kde.org/projects/kde/kdesdk/kompare/repository/revisions/master/entry/libdialogpages/diffpage.cpp#L45)
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 6df9015 
> 
> Diff: https://git.reviewboard.kde.org/r/122556/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun AK
> 
>

Re: Review Request 122556: Bump Qt version to 5.4

[Kevin Funk]

> On Feb. 13, 2015, 8:41 a.m., Martin Gräßlin wrote:
> > AFAIK our CI system doesn't support Qt 5.4 yet. I think we should first 
> > ensure the CI system is prepared for building with the new dependency.
> 
> Albert Astals Cid wrote:
> It does
> http://build.kde.org/job/kde-baseapps_master_qt5/238/consoleFull 
>  qt5 - Branch 5.4.1

Are we sure we want to depend on 5.4 just yet for such a central part of the 
KDE workspace? I don't know the policies, but I'm curious.

Qt 5.4 is not even available on Kubuntu Vivid 15.04 yet.


- Kevin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122556/#review75971
---


On Feb. 13, 2015, 8:06 a.m., Arjun AK wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122556/
> ---
> 
> (Updated Feb. 13, 2015, 8:06 a.m.)
> 
> 
> Review request for KDE Base Apps.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> ---
> 
> `QUrl::fromUserInput(userInput, workingDirectory, UserInputResolutionOptions 
> options)` seems to be Qt 5.4+. We either need to bump requirement to 5.4 or 
> do what kompare 
> [did](https://projects.kde.org/projects/kde/kdesdk/kompare/repository/revisions/master/entry/libdialogpages/diffpage.cpp#L45)
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 6df9015 
> 
> Diff: https://git.reviewboard.kde.org/r/122556/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun AK
> 
>

Re: Review Request 122552: warnings--

[Marco Martin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122552/#review75975
---

Ship it!


Ship It!

- Marco Martin


On Feb. 13, 2015, 1:49 a.m., Jeremy Whiting wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122552/
> ---
> 
> (Updated Feb. 13, 2015, 1:49 a.m.)
> 
> 
> Review request for Bodega, kdelibs, Aaron J. Seigo, and Marco Martin.
> 
> 
> Repository: bodega-server
> 
> 
> Description
> ---
> 
> warnings--
> 
> 
> Diffs
> -
> 
>   server/app.js 4217a67735218e6bf1ccb04696a4e650319bb5d0 
> 
> Diff: https://git.reviewboard.kde.org/r/122552/diff/
> 
> 
> Testing
> ---
> 
> This fixes a  few more warnings seen at runtime, with this fix browsing to 
> localhost:3000/contact (or any other url in the api) shows the 404 page.
> Without this fix it shows nothing and never responds.
> 
> 
> Thanks,
> 
> Jeremy Whiting
> 
>

Re: Review Request 122556: Bump Qt version to 5.4

[Aleix Pol Gonzalez]

> On Feb. 13, 2015, 9:41 a.m., Martin Gräßlin wrote:
> > AFAIK our CI system doesn't support Qt 5.4 yet. I think we should first 
> > ensure the CI system is prepared for building with the new dependency.
> 
> Albert Astals Cid wrote:
> It does
> http://build.kde.org/job/kde-baseapps_master_qt5/238/consoleFull 
>  qt5 - Branch 5.4.1
> 
> Kevin Funk wrote:
> Are we sure we want to depend on 5.4 just yet for such a central part of 
> the KDE workspace? I don't know the policies, but I'm curious.
> 
> Qt 5.4 is not even available on Kubuntu Vivid 15.04 yet.

Plasma 5.2 recommends Qt 5.4 already, if it wasn't because of the build.kde.org 
issue, I'd say it would be requiring 5.4.

+1 to raising the dependency version.


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122556/#review75971
---


On Feb. 13, 2015, 9:06 a.m., Arjun AK wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122556/
> ---
> 
> (Updated Feb. 13, 2015, 9:06 a.m.)
> 
> 
> Review request for KDE Base Apps.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> ---
> 
> `QUrl::fromUserInput(userInput, workingDirectory, UserInputResolutionOptions 
> options)` seems to be Qt 5.4+. We either need to bump requirement to 5.4 or 
> do what kompare 
> [did](https://projects.kde.org/projects/kde/kdesdk/kompare/repository/revisions/master/entry/libdialogpages/diffpage.cpp#L45)
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 6df9015 
> 
> Diff: https://git.reviewboard.kde.org/r/122556/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun AK
> 
>

Re: Review Request 122552: warnings--

[Jeremy Whiting]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122552/
---

(Updated Feb. 13, 2015, 1:17 p.m.)


Status
--

This change has been marked as submitted.


Review request for Bodega, kdelibs, Aaron J. Seigo, and Marco Martin.


Repository: bodega-server


Description
---

warnings--


Diffs
-

  server/app.js 4217a67735218e6bf1ccb04696a4e650319bb5d0 

Diff: https://git.reviewboard.kde.org/r/122552/diff/


Testing
---

This fixes a  few more warnings seen at runtime, with this fix browsing to 
localhost:3000/contact (or any other url in the api) shows the 404 page.
Without this fix it shows nothing and never responds.


Thanks,

Jeremy Whiting

KPeople part of KDE Frameworks

[Aleix Pol]

Hi,
I would like to submit KPeople for review process so it can join the
KF5 family in the next 5.8 release.

KPeople is a tier 3 framework (because of KService, which will be
dropped eventually, without breaking ABI) and it offers cross-source
contact aggregation. More information about it can be read on the
README [1].

Your reviews will be welcome.

Thanks!
Aleix

[1] 
https://projects.kde.org/projects/playground/network/kpeople/repository/revisions/master/entry/README.md

Re: KDiagram libs (KChart, KGantt) in KDE Review

[Friedrich W. H. Kossebau]

Am Mittwoch, 11. Februar 2015, 16:30:39 schrieb Andreas Hartmetz:
> On Wednesday 11 February 2015 14:59:50 Adriaan de Groot wrote:
> > On Monday 09 February 2015 01:50:03 Friedrich W. H. Kossebau wrote:
> > > Yes, nearly copy&paste: the copies of KDChart in Calligra & KMyMoney
> > > are older  (2.4.1, based on Qt4) versions, while the copy of
> > > KDChart in Massif-Visualizer matches the version of the KChart lib
> > > in KDiagram.
> > 
> > I've tried compiling the code on FreeBSD 10.1-RELEASE with Clang 3.4.1
> > (I'm assuming that's a supported compiler -- on techbase, searching
> > for "supported compiler" doesn't give me much compatibility
> > information newer than KDE 4.2).
> > 
> >  - I need to add /usr/local/include to the include search path; this
> > 
> > is not kdiagram specific, but seems to be a general Qt5 issue on
> > FreeBSD.
> > 
> >  - TestDrawIntoPainter seems to hang; after 2 min at 100% CPU I killed
> > 
> > it. I ran it separately by hand and get output about missing OpenGL
> > drivers (which is true, I'm building kdiagram in a restricted
> > environment; I didn't originally expect to need to have DBUS running
> > 
> > to be able to do the tests either) and debug output like:
> > QDEBUG : TestDrawIntoPainter::testTest() Time for drawing pixmap
> > :
> > :/test: 53682 ms
> > :
> >   Is that test supposed to take so much longer (minutes) than the
> > 
> > other tests (deciseconds)?
> 
> I guess so. The test is commented out in the .pro file in the KDAB
> version - the reason is probably that it takes so long.

Yes, this test is also aborted on CI, see 
http://build.kde.org/job/kdiagram_master_qt5/lastCompletedBuild/testReport/

And locally also ran forever=until I cancelled. Will disable then as well in 
KDiagram. And perhaps investigate for a possible error in the code, if I feel 
more curious later :)

> > So from a it-compiles-and-the-tests-pass point of view on my platform,
> > it looks good.

Thanks again for that testing, Ade, should have helped by that one fix for 
clean builds with clang in general :)

Cheers
Friedrich

Re: KPeople part of KDE Frameworks

[Aleix Pol]

On Fri, Feb 13, 2015 at 2:25 PM, Aleix Pol  wrote:
> Hi,
> I would like to submit KPeople for review process so it can join the
> KF5 family in the next 5.8 release.
>
> KPeople is a tier 3 framework (because of KService, which will be
> dropped eventually, without breaking ABI) and it offers cross-source
> contact aggregation. More information about it can be read on the
> README [1].
>
> Your reviews will be welcome.
>
> Thanks!
> Aleix
>
> [1] 
> https://projects.kde.org/projects/playground/network/kpeople/repository/revisions/master/entry/README.md

Hi,
Somebody requested generated API documentation for easier review, I
just ran kapidox on kpeople, hope this helps:
http://proli.net/meu/kpeople/apidocs/html/

Aleix

Review Request 122563: [OS X] emulate native window settings restore (size and position)

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122563/
---

Review request for KDE Software on Mac OS X and kdelibs.


Repository: kdelibs


Description
---

OS X applications usually remember the last used screen, size and screen 
position across restarts, and do not save screen-resolution dependent settings. 

The patch proposed here emulates that behaviour by following the example given 
in the documentation for `QWidget::saveGeometry`: the current geometry is saved 
in a key called `geometry`, and position is kept up to date by letting 
`QEvent::Move` dirty the size setting, as under MS Windows.

I think it's good practice to support previously stored settings when changing 
a config store, so the code attempts to restore "legacy" settings when the new 
key is unavailable.


Diffs
-

  kdeui/widgets/kmainwindow.cpp 85beacc 

Diff: https://git.reviewboard.kde.org/r/122563/diff/


Testing
---

On OS X 10.9.5 with Qt 4.8.6 and kdelibs 4.14.5 (head of the 4.14 branch); 
tested with systemsettings, kate and kontact.


Thanks,

René J.V. Bertin

Re: Review Request 122563: [OS X] emulate native window settings restore (size and position)

[Jeremy Whiting]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122563/#review76003
---

Ship it!


Ship It!

- Jeremy Whiting


On Feb. 13, 2015, 12:19 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122563/
> ---
> 
> (Updated Feb. 13, 2015, 12:19 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kdelibs.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> ---
> 
> OS X applications usually remember the last used screen, size and screen 
> position across restarts, and do not save screen-resolution dependent 
> settings. 
> 
> The patch proposed here emulates that behaviour by following the example 
> given in the documentation for `QWidget::saveGeometry`: the current geometry 
> is saved in a key called `geometry`, and position is kept up to date by 
> letting `QEvent::Move` dirty the size setting, as under MS Windows.
> 
> I think it's good practice to support previously stored settings when 
> changing a config store, so the code attempts to restore "legacy" settings 
> when the new key is unavailable.
> 
> 
> Diffs
> -
> 
>   kdeui/widgets/kmainwindow.cpp 85beacc 
> 
> Diff: https://git.reviewboard.kde.org/r/122563/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9.5 with Qt 4.8.6 and kdelibs 4.14.5 (head of the 4.14 branch); 
> tested with systemsettings, kate and kontact.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 122552: warnings--

[Marco Martin]

I did some tests.
the correct url is http://localhost:3000/bodega/v1/json/route  even tough
no routes seems to match

I can't figure out why, the only thing seems to be the express module to be
broken in some way.
probably good to make some tests with simple exampl-ish express apps


On Fri, Feb 13, 2015 at 2:17 PM, Jeremy Whiting  wrote:

>This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122552/
>   This change has been marked as submitted.
>   Review request for Bodega, kdelibs, Aaron J. Seigo, and Marco Martin.
> By Jeremy Whiting.
>
> *Updated Feb. 13, 2015, 1:17 p.m.*
>  *Repository: * bodega-server
> Description
>
> warnings--
>
>   Testing
>
> This fixes a  few more warnings seen at runtime, with this fix browsing to 
> localhost:3000/contact (or any other url in the api) shows the 404 page.
> Without this fix it shows nothing and never responds.
>
>   Diffs
>
>- server/app.js (4217a67735218e6bf1ccb04696a4e650319bb5d0)
>
> View Diff 
>

Re: Review Request 122563: [OS X] emulate native window settings restore (size and position)

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122563/
---

(Updated Feb. 13, 2015, 8:47 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Software on Mac OS X and kdelibs.


Repository: kdelibs


Description
---

OS X applications usually remember the last used screen, size and screen 
position across restarts, and do not save screen-resolution dependent settings. 

The patch proposed here emulates that behaviour by following the example given 
in the documentation for `QWidget::saveGeometry`: the current geometry is saved 
in a key called `geometry`, and position is kept up to date by letting 
`QEvent::Move` dirty the size setting, as under MS Windows.

I think it's good practice to support previously stored settings when changing 
a config store, so the code attempts to restore "legacy" settings when the new 
key is unavailable.


Diffs
-

  kdeui/widgets/kmainwindow.cpp 85beacc 

Diff: https://git.reviewboard.kde.org/r/122563/diff/


Testing
---

On OS X 10.9.5 with Qt 4.8.6 and kdelibs 4.14.5 (head of the 4.14 branch); 
tested with systemsettings, kate and kontact.


Thanks,

René J.V. Bertin

Re: Review Request 122556: Bump Qt version to 5.4

[Ben Cooksley]

> On Feb. 13, 2015, 8:41 a.m., Martin Gräßlin wrote:
> > AFAIK our CI system doesn't support Qt 5.4 yet. I think we should first 
> > ensure the CI system is prepared for building with the new dependency.
> 
> Albert Astals Cid wrote:
> It does
> http://build.kde.org/job/kde-baseapps_master_qt5/238/consoleFull 
>  qt5 - Branch 5.4.1
> 
> Kevin Funk wrote:
> Are we sure we want to depend on 5.4 just yet for such a central part of 
> the KDE workspace? I don't know the policies, but I'm curious.
> 
> Qt 5.4 is not even available on Kubuntu Vivid 15.04 yet.
> 
> Aleix Pol Gonzalez wrote:
> Plasma 5.2 recommends Qt 5.4 already, if it wasn't because of the 
> build.kde.org issue, I'd say it would be requiring 5.4.
> 
> +1 to raising the dependency version.

As Albert mentioned above, the CI system already provides Qt 5.4.1.


- Ben


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122556/#review75971
---


On Feb. 13, 2015, 8:06 a.m., Arjun AK wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122556/
> ---
> 
> (Updated Feb. 13, 2015, 8:06 a.m.)
> 
> 
> Review request for KDE Base Apps.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> ---
> 
> `QUrl::fromUserInput(userInput, workingDirectory, UserInputResolutionOptions 
> options)` seems to be Qt 5.4+. We either need to bump requirement to 5.4 or 
> do what kompare 
> [did](https://projects.kde.org/projects/kde/kdesdk/kompare/repository/revisions/master/entry/libdialogpages/diffpage.cpp#L45)
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 6df9015 
> 
> Diff: https://git.reviewboard.kde.org/r/122556/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun AK
> 
>

Re: Review Request 122556: Bump Qt version to 5.4

[Marko Käning]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122556/#review76012
---


I'd vote for keeping it compatible to Qt 5.3, if that's possible.

- Marko Käning


On Feb. 13, 2015, 9:06 a.m., Arjun AK wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122556/
> ---
> 
> (Updated Feb. 13, 2015, 9:06 a.m.)
> 
> 
> Review request for KDE Base Apps.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> ---
> 
> `QUrl::fromUserInput(userInput, workingDirectory, UserInputResolutionOptions 
> options)` seems to be Qt 5.4+. We either need to bump requirement to 5.4 or 
> do what kompare 
> [did](https://projects.kde.org/projects/kde/kdesdk/kompare/repository/revisions/master/entry/libdialogpages/diffpage.cpp#L45)
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 6df9015 
> 
> Diff: https://git.reviewboard.kde.org/r/122556/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun AK
> 
>

Re: Review Request 122556: Bump Qt version to 5.4

[Inge Wallin]

> On Feb. 13, 2015, 11:39 p.m., Marko Käning wrote:
> > I'd vote for keeping it compatible to Qt 5.3, if that's possible.

+1. Depending on 5.4 in applications already is bad enough even though it can 
sometimes be necessary but I am against it for our base stuff. Especially in 
this case where there is a workaround.


- Inge


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122556/#review76012
---


On Feb. 13, 2015, 9:06 a.m., Arjun AK wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122556/
> ---
> 
> (Updated Feb. 13, 2015, 9:06 a.m.)
> 
> 
> Review request for KDE Base Apps.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> ---
> 
> `QUrl::fromUserInput(userInput, workingDirectory, UserInputResolutionOptions 
> options)` seems to be Qt 5.4+. We either need to bump requirement to 5.4 or 
> do what kompare 
> [did](https://projects.kde.org/projects/kde/kdesdk/kompare/repository/revisions/master/entry/libdialogpages/diffpage.cpp#L45)
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 6df9015 
> 
> Diff: https://git.reviewboard.kde.org/r/122556/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun AK
> 
>