[Differential] [Updated] D4363: Don't set gnu style parameter with Clang and MSVC

2017-01-31 Thread David Faure 
dfaure added a comment.


  I'm no expert but this seems ok, no objection from me.

REPOSITORY
  R240 Extra CMake Modules

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: vonreth, #windows, bcooksley, alexmerry, dfaure
Cc: #frameworks, #build_system

Re: Phabricator: All repositories registered - upcoming workflow changes

2017-01-31 Thread René J . V . Bertin 
On Tuesday January 31 2017 20:10:42 Luigi Toscano wrote:

>> It will be a complete shutdown of Reviewboard - we'll be archiving it
>> in the event for some reason it becomes necessary to access the data
>> it stores.
>
>Isn't it a way to change the site in static website and keep it alive?
>Checking the history of a review can tell a lot. Also for discarded reviews.

I'd vote for that too, because

>> In most cases mailing lists should have the history of reviews in
>> their archives, so those will continue to be accessible through list
>> archives in the long run.

That is true for the reviews, but not for the patches themselves. In my 
experience you get at most the extract reviewers commented on, and that was 
usually only a single line.

It can also be very useful to look at older versions of reviews. That kind of 
history isn't available elsewhere, at least not without significant digging 
around.
What also doesn't help is the fact that the email notifications that are 
archived via mailing lists each contain a big part if not the entire 
review/comment history.

Keeping the website alive as a read-only resource also makes it possible to 
download patchfiles and other resources that were added to reviews (which are 
*not* available via mailing list archives).
And last but not least: knowing myself I'm quite likely (and surely not the 
only one) to forget transfering open reviews to Phabricator before the 
transition is supposed to be final. 

In fact, even if no one forgets a single outstanding RR until it's too late, 
are we supposed to copy all review comments, or are reviewers supposed to take 
to mailing list archives to consult the review history? Both options aren't 
really acceptable if you ask me so unless there's an automatic 
transfer+conversion process this seems like an important argument to keep the 
reviewboard site around.

R.

Re: Phabricator: All repositories registered - upcoming workflow changes

2017-01-31 Thread Luigi Toscano 
Ben Cooksley ha scritto:
> On Tue, Jan 31, 2017 at 11:36 PM, René J.V. Bertin  
> wrote:
>> On Sunday January 29 2017 08:32:21 Ben Cooksley wrote:
>>
>> Hi,
> 
> Hi Rene,
> 
>>
>> >From this point forward, communities should be moving away from
>>> Reviewboard to Phabricator for conducting code review. Sysadmin will
>>> be announcing a timeline for the shutdown of Reviewboard in the near
>>> future.
>>
>> I hope that shutdown doesn't mean complete disconnect; it would probably be 
>> a loss of as-yet unknown importance if all code reviews become unavailable.
>>
>> I'll miss ReviewBoard. Phabrithingy may be more powerful and versatile, but 
>> RB had its advantages too which could be why it's still being used (quite a 
>> lot, as far as I can see) and hasn't been integrated with KDE's own IDE yet.
> 
> It will be a complete shutdown of Reviewboard - we'll be archiving it
> in the event for some reason it becomes necessary to access the data
> it stores.

Isn't it a way to change the site in static website and keep it alive?
Checking the history of a review can tell a lot. Also for discarded reviews.

> 
> In most cases mailing lists should have the history of reviews in
> their archives, so those will continue to be accessible through list
> archives in the long run.

I think we should ensure to have the archives on mail.kde.org if this is the 
case.


-- 
Luigi

Re: Phabricator: All repositories registered - upcoming workflow changes

2017-01-31 Thread Ben Cooksley 
On Tue, Jan 31, 2017 at 11:36 PM, René J.V. Bertin  wrote:
> On Sunday January 29 2017 08:32:21 Ben Cooksley wrote:
>
> Hi,

Hi Rene,

>
> >From this point forward, communities should be moving away from
>>Reviewboard to Phabricator for conducting code review. Sysadmin will
>>be announcing a timeline for the shutdown of Reviewboard in the near
>>future.
>
> I hope that shutdown doesn't mean complete disconnect; it would probably be a 
> loss of as-yet unknown importance if all code reviews become unavailable.
>
> I'll miss ReviewBoard. Phabrithingy may be more powerful and versatile, but 
> RB had its advantages too which could be why it's still being used (quite a 
> lot, as far as I can see) and hasn't been integrated with KDE's own IDE yet.

It will be a complete shutdown of Reviewboard - we'll be archiving it
in the event for some reason it becomes necessary to access the data
it stores.

In most cases mailing lists should have the history of reviews in
their archives, so those will continue to be accessible through list
archives in the long run.


>
> R.

Cheers,
Ben

[Differential] [Updated] D4377: Fix Python dependency in test scripts for KFileMetaData (bug 375472)

2017-01-31 Thread A. Wilcox 
awilcox updated the test plan for this revision.

REPOSITORY
  R286 KFileMetaData

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: awilcox
Cc: #frameworks

[Differential] [Request, 10 lines] D4377: Fix Python dependency in test scripts for KFileMetaData (bug 375472)

2017-01-31 Thread A. Wilcox 
awilcox created this revision.
awilcox set the repository for this revision to R286 KFileMetaData.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  KFileMetaData test scripts are usable in both deprecated Python 2, and new 
Python 3.  This removes 'python2' from the shebang and uses the regular 
'python' interpreter, thereby removing the requirement of Python 2 being on a 
system to be able to test KFileMetaData.
  
  Additionally, it removes unused imports.

TEST PLAN
  This patch was applied to:
  
  - 5.23
  - 5.26
  - 5.29
  
  It was tested on Linux kernel 4.4 using:
  
  - x86_64
  - x86_32
  - PowerPC
  
  It was tested on glibc and musl libc.

REPOSITORY
  R286 KFileMetaData

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

AFFECTED FILES
  kfilemetadata-5.23.0/autotests/samplefiles/testexternalextractor/main.py
  kfilemetadata-5.23.0/autotests/samplefiles/testexternalwriter/main.py

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: awilcox
Cc: #frameworks

KConfig issues prevent compiling KDE applications under Windows

2017-01-31 Thread Jasem Mutlaq 
Hello,

KConfig used to work perfectly fine under Windows. I recently tried to
compile KStars under Windows 10 (64bit) with MSVC 2015 and Qt 5.8 using
Craft, but encountered an issue as explained in this bug report:

https://bugs.kde.org/show_bug.cgi?id=375654

I talked with Craft maintainers (Hannah et al) and they told me this was an
issue from KConfig side, not Craft. Can someone please look into this and
fix it as it our release is dependent on this.

-- 
Best Regards,
Jasem Mutlaq

[Differential] [Request, 88 lines] D4365: Make it possible to adopt resources, mostly for system-wide settings

2017-01-31 Thread Aleix Pol Gonzalez 
apol created this revision.
apol added reviewers: Frameworks, Plasma, mart.
Restricted Application added projects: Plasma, Frameworks.
Restricted Application added a subscriber: plasma-devel.

REVISION SUMMARY
  Makes it possible to specify commands that will put the resource in use, such
  as changing the wallpaper or changing the icon theme or color scheme.

TEST PLAN
  tested with some plasma resources, see patches against plasma-workspace and 
plasma-desktop.

REPOSITORY
  R304 KNewStuff

BRANCH
  master

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

AFFECTED FILES
  src/core/engine.cpp
  src/core/engine.h
  src/core/installation.cpp
  src/core/installation.h
  src/ui/itemsviewbasedelegate.cpp
  src/ui/itemsviewdelegate.cpp

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: apol, #frameworks, #plasma, mart
Cc: plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas

Jenkins-kde-ci: breeze-icons master stable-kf5-qt5 » Linux,gcc - Build # 485 - Still Failing!

2017-01-31 Thread no-reply 

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/breeze-icons%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/485/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 31 Jan 2017 12:05:05 +
Build duration: 22 sec

CHANGE SET
Revision 3d61131ce072366a9b95ae6e598760903d2b5769 by Harald Sitter: (only look 
for test dependencies when testing)
  change: edit autotests/CMakeLists.txt

Jenkins-kde-ci: breeze-icons master kf5-qt5 » Linux,gcc - Build # 485 - Still Failing!

2017-01-31 Thread no-reply 

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/breeze-icons%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/485/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 31 Jan 2017 12:05:05 +
Build duration: 23 sec

CHANGE SET
Revision 3d61131ce072366a9b95ae6e598760903d2b5769 by Harald Sitter: (only look 
for test dependencies when testing)
  change: edit autotests/CMakeLists.txt

Jenkins-kde-ci: breeze-icons master kf5-qt5 » Linux,gcc - Build # 484 - Failure!

2017-01-31 Thread no-reply 

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/breeze-icons%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/484/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 31 Jan 2017 11:50:40 +
Build duration: 26 sec

CHANGE SET
Revision 7d01f3d8b5c0f2c9dc6014e8e04ba5701804fec2 by Harald Sitter: (add new 
test for scalable exposure)
  change: add autotests/scalabletest.cpp
  change: edit autotests/CMakeLists.txt

Jenkins-kde-ci: breeze-icons master stable-kf5-qt5 » Linux,gcc - Build # 484 - Failure!

2017-01-31 Thread no-reply 

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/breeze-icons%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/484/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 31 Jan 2017 11:50:39 +
Build duration: 42 sec

CHANGE SET
Revision 7d01f3d8b5c0f2c9dc6014e8e04ba5701804fec2 by Harald Sitter: (add new 
test for scalable exposure)
  change: edit autotests/CMakeLists.txt
  change: add autotests/scalabletest.cpp

[Differential] [Closed] D4254: add new test for scalable exposure

2017-01-31 Thread Harald Sitter 
This revision was automatically updated to reflect the committed changes.
Closed by commit R266:7d01f3d8b5c0: add new test for scalable exposure 
(authored by sitter).

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4254?vs=10460=10757

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/scalabletest.cpp

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: sitter
Cc: #frameworks

[Differential] [Updated] D4363: Don't set gnu style parameter with Clang and MSVC

2017-01-31 Thread Hannah von Reth 
vonreth added reviewers: Windows, bcooksley, alexmerry, dfaure.

REPOSITORY
  R240 Extra CMake Modules

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: vonreth, #windows, bcooksley, alexmerry, dfaure
Cc: #frameworks, #build_system

[Differential] [Request, 38 lines] D4363: Don't set gnu style parameter with Clang and MSVC

2017-01-31 Thread Hannah von Reth 
vonreth created this revision.
Restricted Application added projects: Frameworks, Build System.
Restricted Application added subscribers: Build System, Frameworks.

REVISION SUMMARY
  Setting the gnu style compiler flags with clang-cl results in a
  huge bunch of unknown argument warnings.
  
  The patch just tests for Clang AND NOT MSVC, while we might wan't
  a more fine gained test for clang-cl, as in theory it could be
  possible to use a plain clang++ with msvc.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

AFFECTED FILES
  kde-modules/KDECompilerSettings.cmake
  kde-modules/KDEFrameworkCompilerSettings.cmake

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: vonreth
Cc: #frameworks, #build_system

[Differential] [Commented On] D4362: RFC: [AppletQuickItem] Cache QQmlComponent used for settings QtQuick Controls 1 style

2017-01-31 Thread David Edmundson 
davidedmundson added a comment.


  Personally I would just kill the whole thing.
  
  - All Plasma code all uses Plasma Components not QQC so this has zero effect. 
It was for an idea that didn't really materialise
  - It gives an obscure ASAN warning on freeing "o", that I don't know how to 
fix
  - This isn't going to work with QQC2

INLINE COMMENTS

> appletquickitem.cpp:496
> +if (!c) {
> +c = new QQmlComponent(engine);
> +// FIXME never executed, even if I kquitapp plasmashell

if you were to do this:
just do c = new QQmlComponent(qApp);

and you have the cleanup done for you.

REPOSITORY
  R242 Plasma Framework (Library)

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: broulik, #plasma
Cc: davidedmundson, plasma-devel, #frameworks, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas

[Differential] [Request, 31 lines] D4362: RFC: [AppletQuickItem] Cache QQmlComponent used for settings QtQuick Controls 1 style

2017-01-31 Thread Kai Uwe Broulik 
broulik created this revision.
broulik added a reviewer: Plasma.
broulik set the repository for this revision to R242 Plasma Framework (Library).
Restricted Application added projects: Plasma, Frameworks.
Restricted Application added subscribers: Frameworks, plasma-devel.

REVISION SUMMARY
  Plasma spends roughly 0.5ms per applet just compiling this component 
(QQmlComponent creation), so cache it away.
  All applets did share the same QQmlEngine here but I don't think this can be 
trusted, hence store the components in a QHash per engine.
  While at it, turn the Item into a QtObject to reduce overhead since we just 
want the Component.onCompleted to be executed and that's it.
  
  Before: 41 calls (18ms in total)
  After: 41 calls (1.9ms in total)

TEST PLAN
  Verified that a QtQuick Controls 1 Button still uses the Plasma theme.
  
  The QObject::destroyed I use to remove the component from the hash again is 
never executed, even on kquitapp plasmashell.

REPOSITORY
  R242 Plasma Framework (Library)

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

AFFECTED FILES
  src/plasmaquick/appletquickitem.cpp
  src/plasmaquick/private/appletquickitem_p.h

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: broulik, #plasma
Cc: plasma-devel, #frameworks, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas

Re: Phabricator: All repositories registered - upcoming workflow changes

2017-01-31 Thread René J . V . Bertin 
On Sunday January 29 2017 08:32:21 Ben Cooksley wrote:

Hi,

>From this point forward, communities should be moving away from
>Reviewboard to Phabricator for conducting code review. Sysadmin will
>be announcing a timeline for the shutdown of Reviewboard in the near
>future.

I hope that shutdown doesn't mean complete disconnect; it would probably be a 
loss of as-yet unknown importance if all code reviews become unavailable.

I'll miss ReviewBoard. Phabrithingy may be more powerful and versatile, but RB 
had its advantages too which could be why it's still being used (quite a lot, 
as far as I can see) and hasn't been integrated with KDE's own IDE yet.

R.

[Differential] [Commented On] D4361: [Containment] Move contains() check inside NDEBUG

2017-01-31 Thread Bhushan Shah 
bshah added a comment.


  I'd say we should port away all of the users of NDEBUG to qCDebug (if 
anything is still using qDebug) and remove those ifdefs..

REPOSITORY
  R242 Plasma Framework (Library)

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: broulik, #plasma
Cc: bshah, plasma-devel, #frameworks, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas

[Differential] [Request, 4 lines] D4361: [Containment] Move contains() check inside NDEBUG

2017-01-31 Thread Kai Uwe Broulik 
broulik created this revision.
broulik added a reviewer: Plasma.
broulik set the repository for this revision to R242 Plasma Framework (Library).
Restricted Application added projects: Plasma, Frameworks.
Restricted Application added subscribers: Frameworks, plasma-devel.

REVISION SUMMARY
  Since we don't return and only do something when NDEBUG is defined, don't do 
contains() for no reason.

TEST PLAN
  Not sure how useful this thing even is since the qCDebug inside the define is 
commented, even..

REPOSITORY
  R242 Plasma Framework (Library)

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

AFFECTED FILES
  src/plasma/containment.cpp

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: broulik, #plasma
Cc: plasma-devel, #frameworks, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas