Re: Finding contributor email is imposible - was - Re: Phabricator: All repositories registered - upcoming workflow changes

2017-02-07 Thread Ben Cooksley 
On Wed, Feb 8, 2017 at 4:03 AM, Fredrik Höglund  wrote:
> On Monday 06 February 2017, Albert Astals Cid wrote:
>> El diumenge, 29 de gener de 2017, a les 8:32:21 CET, Ben Cooksley va 
>> escriure:
>> > Hi everyone,
>> >
>> > 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.
>>
>> Today i wanted to commit https://phabricator.kde.org/D4432 since the person
>> that opened it does not have a developer account.
>>
>> Easy peasy, it's just a line, i download the patch and then
>>
>> git commit --author ...
>>
>> and what, it says "Authored by rikmills" that's probably not the guys name,
>> but i can click on the name and on https://phabricator.kde.org/p/rikmills/ i
>> learn he is Rik Mills, good.
>>
>> $ git commit --author=="Rik Mills"
>> fatal: --author 'Rik Mills' is not 'Name ' and matches no existing
>> author
>>
>> ouch, so i need his email, where do i get it?
>>
>> Nowhere it seems.
>>
>> I resorted to searching for it in identity.kde.org but i think that i can 
>> only
>> do that because i have some special power over there that allows me to see
>> everyone's email, and even if everyone can, it's very cumbsersome, and
>> probably what i would end up doing is asking in Differential, the author 
>> would
>> have to answer, potentially either him or me forgetting about it.
>>
>> On reviewboard it was as simple as going to
>> https://git.reviewboard.kde.org/users/rikmills/ (that i just realized i could
>> have used instead of identity :D but it's going away so it's not a solution
>> either).
>
> This is the wrong solution.  Phabricator should provide the patch in a format
> that you can apply to the repository with git am -s , and get the
> original commit message, date and author.
>
> You should never have to enter the author or commit message yourself
> when you are committing something for someone else.

Phabricator also supports Subversion and Mercurial repositories, along
with reviews for which a repository is not set.
It's also debatable which fields from the review should end up in the
commit message.

>
> Fredrik
>

Regards,
Ben

Re: Finding contributor email is imposible - was - Re: Phabricator: All repositories registered - upcoming workflow changes

2017-02-07 Thread Ben Cooksley 
On Tue, Feb 7, 2017 at 3:30 PM, Michael Pyne  wrote:
> On Mon, Feb 06, 2017 at 11:35:15PM +0100, Albert Astals Cid wrote:
>> El diumenge, 29 de gener de 2017, a les 8:32:21 CET, Ben Cooksley va 
>> escriure:
>> > Hi everyone,
>> >
>> > 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.
>>
>> Today i wanted to commit https://phabricator.kde.org/D4432 since the person
>> that opened it does not have a developer account.
>>  
>>
>> ouch, so i need his email, where do i get it?
>>
>> Nowhere it seems.
>>
>> I resorted to searching for it in identity.kde.org but i think that i can 
>> only
>> do that because i have some special power over there that allows me to see
>> everyone's email, and even if everyone can, it's very cumbsersome, and
>> probably what i would end up doing is asking in Differential, the author 
>> would
>> have to answer, potentially either him or me forgetting about it.

Hi all,

In relation to this issue please continue this at
https://phabricator.kde.org/T5242
If we could keep the discussion in a single place that would be appreciated.

>
> In fairness to Phabricator there's been times where I've needed
> developer contact info *without* having a RR available.  In the "good
> old days" we'd just use the kde-common/accounts file to lookup the
> developer's id and corresponding email address, and that worked fine.

The kde-common/accounts file is still being maintained, however
Sysadmin does intend to disconnect Subversion usernames from
everything else, to avoid the arguments over usernames we have with
some people.

>
> So I think Identity (and in general whatever our KDE organizational
> directory solution happens to be) is the proper solution -- although it
> shouldn't need superpowers to actually see those identities.  And of
> course, there's no reason not to also show the email in Phabricator,
> which would be more helpful and developer-friendly.

We'll probably only be able to address this when the replacement to
Identity is built, as existing users accepted a privacy policy which
stated we wouldn't publish their details.

We'll have to be careful how this is done however, knowing the privacy
activists we have lurking around.

Sysadmin has received many requests in the past to have Bugzilla
accounts removed because "you're exposing my email address" even
though the site says in a massive font that your email address will be
made public when your register.

>
> Regards,
>  - Michael Pyne

Regards,
Ben

Jenkins-kde-ci: kio master stable-kf5-qt5 » Linux,gcc - Build # 425 - Fixed!

2017-02-07 Thread no-reply 

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kio%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/425/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 07 Feb 2017 17:31:57 +
Build duration: 16 min

CHANGE SET
Revision 726abf9a6b177f80b627f46ff453c272a499d783 by michael: (Fix double 
export in already-exported class)
  change: edit src/widgets/krun.h


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 53 test(s), Skipped: 0 test(s), Total: 
53 test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 21/21 (100%)FILES 274/343 (80%)CLASSES 274/343 (80%)LINE 29742/51615 
(58%)CONDITIONAL 16331/38747 (42%)

By packages
  
autotests
FILES 67/67 (100%)CLASSES 67/67 (100%)LINE 7918/8245 
(96%)CONDITIONAL 4428/8664 (51%)
autotests.http
FILES 9/9 (100%)CLASSES 9/9 (100%)LINE 543/544 
(100%)CONDITIONAL 200/336 (60%)
autotests.kcookiejar
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 179/198 (90%)CONDITIONAL 
60/90 (67%)
src.core
FILES 97/117 (83%)CLASSES 97/117 (83%)LINE 8071/14179 
(57%)CONDITIONAL 4424/9259 (48%)
src.core.kssl
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 35/93 (38%)CONDITIONAL 
3/6 (50%)
src.filewidgets
FILES 26/36 (72%)CLASSES 26/36 (72%)LINE 3448/7559 
(46%)CONDITIONAL 1280/4381 (29%)
src.gui
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 104/110 (95%)CONDITIONAL 
46/72 (64%)
src.ioslaves.file
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 447/849 (53%)CONDITIONAL 
330/749 (44%)
src.ioslaves.http
FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 1758/3780 
(47%)CONDITIONAL 1268/3460 (37%)
src.ioslaves.http.kcookiejar
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 621/782 (79%)CONDITIONAL 
607/839 (72%)
src.ioslaves.trash
FILES 8/10 (80%)CLASSES 8/10 (80%)LINE 715/1139 
(63%)CONDITIONAL 411/833 (49%)
src.ioslaves.trash.tests
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 686/764 (90%)CONDITIONAL 
445/936 (48%)
src.kioslave
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 14/27 (52%)CONDITIONAL 
5/10 (50%)
src.kntlm
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 373/385 (97%)CONDITIONAL 
111/138 (80%)
src.kpasswdserver
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 382/594 (64%)CONDITIONAL 
284/580 (49%)
src.kpasswdserver.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 283/286 (99%)CONDITIONAL 
146/256 (57%)
src.urifilters.fixhost
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 25/34 (74%)CONDITIONAL 
36/54 (67%)
src.urifilters.ikws
FILES 5/10 (50%)CLASSES 5/10 (50%)LINE 242/727 (33%)CONDITIONAL 
150/546 (27%)
src.urifilters.localdomain
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 21/29 (72%)CONDITIONAL 
16/26 (62%)
src.urifilters.shorturi
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 237/266 (89%)CONDITIONAL 
332/412 (81%)
src.widgets
FILES 32/64 (50%)CLASSES 32/64 (50%)LINE 3640/11025 
(33%)CONDITIONAL 1749/7100 (25%)

Jenkins-kde-ci: kio master stable-kf5-qt5 » Linux,gcc - Build # 425 - Fixed!

2017-02-07 Thread no-reply 

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kio%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/425/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 07 Feb 2017 17:31:57 +
Build duration: 16 min

CHANGE SET
Revision 726abf9a6b177f80b627f46ff453c272a499d783 by michael: (Fix double 
export in already-exported class)
  change: edit src/widgets/krun.h


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 53 test(s), Skipped: 0 test(s), Total: 
53 test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 21/21 (100%)FILES 274/343 (80%)CLASSES 274/343 (80%)LINE 29742/51615 
(58%)CONDITIONAL 16331/38747 (42%)

By packages
  
autotests
FILES 67/67 (100%)CLASSES 67/67 (100%)LINE 7918/8245 
(96%)CONDITIONAL 4428/8664 (51%)
autotests.http
FILES 9/9 (100%)CLASSES 9/9 (100%)LINE 543/544 
(100%)CONDITIONAL 200/336 (60%)
autotests.kcookiejar
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 179/198 (90%)CONDITIONAL 
60/90 (67%)
src.core
FILES 97/117 (83%)CLASSES 97/117 (83%)LINE 8071/14179 
(57%)CONDITIONAL 4424/9259 (48%)
src.core.kssl
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 35/93 (38%)CONDITIONAL 
3/6 (50%)
src.filewidgets
FILES 26/36 (72%)CLASSES 26/36 (72%)LINE 3448/7559 
(46%)CONDITIONAL 1280/4381 (29%)
src.gui
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 104/110 (95%)CONDITIONAL 
46/72 (64%)
src.ioslaves.file
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 447/849 (53%)CONDITIONAL 
330/749 (44%)
src.ioslaves.http
FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 1758/3780 
(47%)CONDITIONAL 1268/3460 (37%)
src.ioslaves.http.kcookiejar
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 621/782 (79%)CONDITIONAL 
607/839 (72%)
src.ioslaves.trash
FILES 8/10 (80%)CLASSES 8/10 (80%)LINE 715/1139 
(63%)CONDITIONAL 411/833 (49%)
src.ioslaves.trash.tests
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 686/764 (90%)CONDITIONAL 
445/936 (48%)
src.kioslave
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 14/27 (52%)CONDITIONAL 
5/10 (50%)
src.kntlm
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 373/385 (97%)CONDITIONAL 
111/138 (80%)
src.kpasswdserver
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 382/594 (64%)CONDITIONAL 
284/580 (49%)
src.kpasswdserver.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 283/286 (99%)CONDITIONAL 
146/256 (57%)
src.urifilters.fixhost
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 25/34 (74%)CONDITIONAL 
36/54 (67%)
src.urifilters.ikws
FILES 5/10 (50%)CLASSES 5/10 (50%)LINE 242/727 (33%)CONDITIONAL 
150/546 (27%)
src.urifilters.localdomain
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 21/29 (72%)CONDITIONAL 
16/26 (62%)
src.urifilters.shorturi
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 237/266 (89%)CONDITIONAL 
332/412 (81%)
src.widgets
FILES 32/64 (50%)CLASSES 32/64 (50%)LINE 3640/11025 
(33%)CONDITIONAL 1749/7100 (25%)

Re: Finding contributor email is imposible - was - Re: Phabricator: All repositories registered - upcoming workflow changes

2017-02-07 Thread Fredrik Höglund 
On Monday 06 February 2017, Albert Astals Cid wrote:
> El diumenge, 29 de gener de 2017, a les 8:32:21 CET, Ben Cooksley va escriure:
> > Hi everyone,
> > 
> > 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.
> 
> Today i wanted to commit https://phabricator.kde.org/D4432 since the person 
> that opened it does not have a developer account.
> 
> Easy peasy, it's just a line, i download the patch and then
> 
> git commit --author ...
> 
> and what, it says "Authored by rikmills" that's probably not the guys name, 
> but i can click on the name and on https://phabricator.kde.org/p/rikmills/ i 
> learn he is Rik Mills, good.
> 
> $ git commit --author=="Rik Mills"
> fatal: --author 'Rik Mills' is not 'Name ' and matches no existing 
> author
> 
> ouch, so i need his email, where do i get it?
> 
> Nowhere it seems.
> 
> I resorted to searching for it in identity.kde.org but i think that i can 
> only 
> do that because i have some special power over there that allows me to see 
> everyone's email, and even if everyone can, it's very cumbsersome, and 
> probably what i would end up doing is asking in Differential, the author 
> would 
> have to answer, potentially either him or me forgetting about it.
> 
> On reviewboard it was as simple as going to 
> https://git.reviewboard.kde.org/users/rikmills/ (that i just realized i could 
> have used instead of identity :D but it's going away so it's not a solution 
> either).

This is the wrong solution.  Phabricator should provide the patch in a format
that you can apply to the repository with git am -s , and get the
original commit message, date and author.

You should never have to enter the author or commit message yourself
when you are committing something for someone else.

Fredrik

Jenkins-kde-ci: knotifications master stable-kf5-qt5 » Linux,gcc - Build # 60 - Fixed!

2017-02-07 Thread no-reply 

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/knotifications%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/60/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 07 Feb 2017 13:21:00 +
Build duration: 5 min 45 sec

CHANGE SET
Revision d642e73614375380c37674b184f036fb862b286d by David Edmundson: (Only use 
qApp-gt;desktopFileName() when compiling with Qt gt;= 5.7.0)
  change: edit src/notifybypopup.cpp


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 2 test(s), Skipped: 0 test(s), Total: 2 
test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 2/2 (100%)FILES 18/29 (62%)CLASSES 18/29 (62%)LINE 540/2191 
(25%)CONDITIONAL 237/1359 (17%)

By packages
  
autotests
FILES 3/3 (100%)CLASSES 3/3 (100%)LINE 142/148 (96%)CONDITIONAL 
66/126 (52%)
src
FILES 15/26 (58%)CLASSES 15/26 (58%)LINE 398/2043 
(19%)CONDITIONAL 171/1233 (14%)

Jenkins-kde-ci: knotifications master stable-kf5-qt5 » Linux,gcc - Build # 60 - Fixed!

2017-02-07 Thread no-reply 

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/knotifications%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/60/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 07 Feb 2017 13:21:00 +
Build duration: 5 min 45 sec

CHANGE SET
Revision d642e73614375380c37674b184f036fb862b286d by David Edmundson: (Only use 
qApp-gt;desktopFileName() when compiling with Qt gt;= 5.7.0)
  change: edit src/notifybypopup.cpp


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 2 test(s), Skipped: 0 test(s), Total: 2 
test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 2/2 (100%)FILES 18/29 (62%)CLASSES 18/29 (62%)LINE 540/2191 
(25%)CONDITIONAL 237/1359 (17%)

By packages
  
autotests
FILES 3/3 (100%)CLASSES 3/3 (100%)LINE 142/148 (96%)CONDITIONAL 
66/126 (52%)
src
FILES 15/26 (58%)CLASSES 15/26 (58%)LINE 398/2043 
(19%)CONDITIONAL 171/1233 (14%)

[Differential] [Commented On] D4416: Send desktopfilename as part of notifyByPopup hints

2017-02-07 Thread David Edmundson 
davidedmundson added a comment.


  In https://phabricator.kde.org/D4416#83780, @cfeck wrote:
  
  > QApplication::desktopFileName() was only added in Qt 5.7, while frameworks 
still support Qt 5.6. Please add a Qt version check around the new code.
  
  
  Done, thanks

REPOSITORY
  R289 KNotifications

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

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

To: davidedmundson, #plasma, apol
Cc: cfeck, graesslin, mck182, hein, plasma-devel, #frameworks, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

[Differential] [Closed] D4473: [ScrollViewStyle] Evaluate frameVisible property

2017-02-07 Thread Roman Gilg 
This revision was automatically updated to reflect the committed changes.
Closed by commit R242:eb39b3514130: [ScrollViewStyle] Evaluate frameVisible 
property (authored by subdiff).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4473?vs=11003=11009

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

AFFECTED FILES
  src/declarativeimports/plasmaextracomponents/qml/ScrollArea.qml
  src/declarativeimports/plasmastyle/ScrollViewStyle.qml

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

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

[Differential] [Commented On] D4473: [ScrollViewStyle] Evaluate frameVisible property

2017-02-07 Thread Roman Gilg 
subdiff added a comment.


  In https://phabricator.kde.org/D4473#83779, @hein wrote:
  
  > Aye. I think that's weird
  
  
  I agree.
  
  > Does defaulting the frames to on mean the bottom flash is back too or is 
that still fixed?
  
  Still fixed.
  
  > Can't comment on the ramifications of defaulting it to on + other themes ...
  
  Well, it should be pretty minimal. Because in general the frame component is 
obviously meant as a decoration only. The Plasma style changes this (maybe in a 
weird way) by being dependent on the scroll posibilities and therefore acting 
as a scrolling indicator.

REPOSITORY
  R242 Plasma Framework (Library)

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

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

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

[Differential] [Updated] D4289: Add VLC tray icon

2017-02-07 Thread Yunhe Guo 
guoyunhe added a reviewer: Plasma.

REPOSITORY
  R242 Plasma Framework (Library)

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

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

To: guoyunhe, #plasma:_design, #plasma
Cc: andreaska, guoyunhe, #frameworks

[Differential] [Commented On] D4473: [ScrollViewStyle] Evaluate frameVisible property

2017-02-07 Thread Eike Hein 
hein added a comment.


  Aye. I think that's weird, but I'm guessing the Breeze style had no choice 
there.
  
  Does defaulting the frames to on mean the bottom flash is back too or is that 
still fixed?
  
  Can't comment on the ramifications of defaulting it to on + other themes ...

REPOSITORY
  R242 Plasma Framework (Library)

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

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

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

[Differential] [Commented On] D4416: Send desktopfilename as part of notifyByPopup hints

2017-02-07 Thread Christoph Feck 
cfeck added a comment.


  QApplication::desktopFileName() was only added in Qt 5.7, while frameworks 
still support Qt 5.6. Please add a Qt version check around the new code.

REPOSITORY
  R289 KNotifications

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

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

To: davidedmundson, #plasma, apol
Cc: cfeck, graesslin, mck182, hein, plasma-devel, #frameworks, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

[Differential] [Commented On] D4473: [ScrollViewStyle] Evaluate frameVisible property

2017-02-07 Thread Roman Gilg 
subdiff added a comment.


  The scroll indicators **are** the frame component in the Breeze style. So 
they get replaced in any other style by the frame specified there. If you take 
a look at ScrollViewStyle.qml, the indicators are inside the frame component 
and the frame sides just change their opacity values

REPOSITORY
  R242 Plasma Framework (Library)

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

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

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

[Differential] [Closed] D4414: don't regenerate frames when setting every property

2017-02-07 Thread Marco Martin 
This revision was automatically updated to reflect the committed changes.
Closed by commit R242:d8a1a9eb084b: don't regenerate frames when setting every 
property (authored by mart).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4414?vs=11006=11007

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

AFFECTED FILES
  autotests/data/background.svgz
  autotests/framesvgtest.cpp
  autotests/framesvgtest.h
  src/declarativeimports/core/framesvgitem.cpp
  src/declarativeimports/core/framesvgitem.h
  src/plasma/framesvg.cpp
  src/plasma/framesvg.h
  src/plasma/private/framesvg_p.h

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

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

[Differential] [Updated, 623 lines] D4414: don't regenerate frames when setting every property

2017-02-07 Thread Marco Martin 
mart updated this revision to Diff 11006.
mart added a comment.


  - just use enabledBorders to return as a property

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4414?vs=10861=11006

BRANCH
  arcpatch-D4414

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

AFFECTED FILES
  autotests/data/background.svgz
  autotests/framesvgtest.cpp
  autotests/framesvgtest.h
  src/declarativeimports/core/framesvgitem.cpp
  src/declarativeimports/core/framesvgitem.h
  src/plasma/framesvg.cpp
  src/plasma/framesvg.h
  src/plasma/private/framesvg_p.h

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

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

[Differential] [Commented On] D4473: [ScrollViewStyle] Evaluate frameVisible property

2017-02-07 Thread Eike Hein 
hein added a comment.


  Is there some danger in this when using the component with a different theme 
that has visible frames?
  
  Should the scroll indicators be tied to frame visibility at all? That seems 
dubious to me.

REPOSITORY
  R242 Plasma Framework (Library)

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

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

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

[Differential] [Commented On] D4414: don't regenerate frames when setting every property

2017-02-07 Thread David Edmundson 
davidedmundson added inline comments.

INLINE COMMENTS

> mart wrote in framesvg.cpp:136
> it has pendingEnabledBorders because right now the borders are saved only n 
> the frame, that we don't know if we can keep it or we'll have to throw it 
> away ( or just dereference because some other framesvg instance still needs 
> it)
> I don't like it that much as well, but i don't think the new value can be 
> assigned right away.
> and yes, when repaintblocked is true, it would return the old value... unless 
> it would return pendingEnabledBorders in this case

That part makes sense now.

We still need to do something, otherwise if I have a Binding on a FrameSVGItem 
it's going to be broken.

I think we can just return d->pendingEnabledBorders rather than 
frame->enabledBorders (possibly renaming it)

the frame will never be different and it solves that problem simply.

REPOSITORY
  R242 Plasma Framework (Library)

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

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

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

[Differential] [Closed] D4288: Add Kleopatra tray icon

2017-02-07 Thread Yunhe Guo 
guoyunhe closed this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

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

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

[Differential] [Updated, 3 lines] D4473: [ScrollViewStyle] Evaluate frameVisible property

2017-02-07 Thread Roman Gilg 
subdiff updated this revision to Diff 11003.
subdiff added a comment.


  Set ScrollArea's frameVisible default to true additional in order to not 
change current implementations.

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4473?vs=10997=11003

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

AFFECTED FILES
  src/declarativeimports/plasmaextracomponents/qml/ScrollArea.qml
  src/declarativeimports/plasmastyle/ScrollViewStyle.qml

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

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

[Differential] [Commented On] D4414: don't regenerate frames when setting every property

2017-02-07 Thread Marco Martin 
mart added inline comments.

INLINE COMMENTS

> davidedmundson wrote in framesvg.cpp:136
> I'm lost on why we have the pendingEnabledBorders
> 
> you have some bugs if you do:
> 
> setRepaintBlocked(false);
> setEnabledBorders(Left)
> enabledBorders() /// returns All not Left.
> 
> it'll get updated later but after the changed signal on the FrameSvgItem.
> 
> Do we need to know the old borders when we update? If so can we revert this 
> logic to have a d->oldBorders that gets set at the end of the update method?

it has pendingEnabledBorders because right now the borders are saved only n the 
frame, that we don't know if we can keep it or we'll have to throw it away ( or 
just dereference because some other framesvg instance still needs it)
I don't like it that much as well, but i don't think the new value can be 
assigned right away.
and yes, when repaintblocked is true, it would return the old value... unless 
it would return pendingEnabledBorders in this case

REPOSITORY
  R242 Plasma Framework (Library)

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

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

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

[Differential] [Changed Subscribers] D4414: don't regenerate frames when setting every property

2017-02-07 Thread David Edmundson 
davidedmundson added inline comments.

INLINE COMMENTS

> framesvg.cpp:136
>  
> -FrameData *fd = d->frames[d->prefix];
> -
> -const QString oldKey = d->cacheId(fd, d->prefix);
> -const EnabledBorders oldBorders = fd->enabledBorders;
> -fd->enabledBorders = borders;
> -const QString newKey = d->cacheId(fd, d->prefix);
> -fd->enabledBorders = oldBorders;
> -
> -//qCDebug(LOG_PLASMA) << "looking for" << newKey;
> -FrameData *newFd = 
> FrameSvgPrivate::s_sharedFrames[theme()->d].value(newKey);
> -if (newFd) {
> -//qCDebug(LOG_PLASMA) << "FOUND IT!" << newFd->refcount;
> -// we've found a math, so insert that new one and ref it ..
> -newFd->ref(this);
> -d->frames.insert(d->prefix, newFd);
> -
> -//.. then deref the old one and if it's no longer used, get rid of it
> -if (fd->deref(this)) {
> -//const QString oldKey = d->cacheId(fd, d->prefix);
> -//qCDebug(LOG_PLASMA) << "1. Removing it" << oldKey << 
> fd->refcount;
> -FrameSvgPrivate::s_sharedFrames[fd->theme].remove(oldKey);
> -delete fd;
> -}
> +d->pendingEnabledBorders = borders;
>  

I'm lost on why we have the pendingEnabledBorders

you have some bugs if you do:

setRepaintBlocked(false);
setEnabledBorders(Left)
enabledBorders() /// returns All not Left.

it'll get updated later but after the changed signal on the FrameSvgItem.

Do we need to know the old borders when we update? If so can we revert this 
logic to have a d->oldBorders that gets set at the end of the update method?

REPOSITORY
  R242 Plasma Framework (Library)

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

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

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

[Differential] [Commented On] D4473: [ScrollViewStyle] Evaluate frameVisible property

2017-02-07 Thread Roman Gilg 
subdiff added a comment.


  You mean in Plasma's ScrollArea? Would be ok for me. It's a rather random 
deviation from upstream's ScrollView though.
  
  Is this ok for anyone else aswell?

REPOSITORY
  R242 Plasma Framework (Library)

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

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

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

[Differential] [Commented On] D4473: [ScrollViewStyle] Evaluate frameVisible property

2017-02-07 Thread Marco Martin 
mart added a comment.


  what about changing the default to true but still have the binding?

REPOSITORY
  R242 Plasma Framework (Library)

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

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

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

[Differential] [Commented On] D4473: [ScrollViewStyle] Evaluate frameVisible property

2017-02-07 Thread Kai Uwe Broulik 
broulik added a comment.


  -1
  
  I don't get the overflow indicator mark anywhere anymore since frameVisible 
is `false` by default for Plasma ScrollView.

REPOSITORY
  R242 Plasma Framework (Library)

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

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

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

[Differential] [Commented On] D4473: [ScrollViewStyle] Evaluate frameVisible property

2017-02-07 Thread Roman Gilg 
subdiff added a comment.


  Note: This patch changes the default. Since until now the frameVisible 
property wasn't evaluated the frames were always shown.
  
  But since QtQuickControls ScrollView (i.e. Plasma Extra Components ScrollArea 
aswell) has set frameVisible to false by default, they are now not shown 
anymore.
  
  Still it makes sense of course to evaluate the property.

REPOSITORY
  R242 Plasma Framework (Library)

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

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

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

[Differential] [Updated] D4473: [ScrollViewStyle] Evaluate frameVisible property

2017-02-07 Thread Eike Hein 
hein added a comment.


  IIUI this means scroll indicators are now disabled by default unless a UI 
sets frameVisible: true? Marco, is this what you +1'd?

REPOSITORY
  R242 Plasma Framework (Library)

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

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

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

[Differential] [Accepted] D4473: [ScrollViewStyle] Evaluate frameVisible property

2017-02-07 Thread Marco Martin 
mart accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R242 Plasma Framework (Library)

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

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

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

[Differential] [Request, 2 lines] D4473: [ScrollViewStyle] Evaluate frameVisible property

2017-02-07 Thread Roman Gilg 
subdiff created this revision.
subdiff added reviewers: Plasma, mart, hein.
subdiff added a subscriber: hein.
subdiff set the repository for this revision to R242 Plasma Framework (Library).
subdiff added a project: Plasma.
Restricted Application added subscribers: Frameworks, plasma-devel.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  ScrollArea has a property for en/disabling the frame provided by its style. 
Until now the property wasn't evaluated, which besides making it impossible to 
deactivate the frame leaded to visual artifacts (in @hein's simple menu 
flashing frame at the bottom of the page list).

TEST PLAN
  Tested with simple menu.

REPOSITORY
  R242 Plasma Framework (Library)

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

AFFECTED FILES
  src/declarativeimports/plasmastyle/ScrollViewStyle.qml

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

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

[Differential] [Accepted] D4471: Fix KCModule::setAuthAction error checking

2017-02-07 Thread Marco Martin 
mart accepted this revision.
mart added a reviewer: mart.
This revision is now accepted and ready to land.

REPOSITORY
  R265 KConfigWidgets

BRANCH
  master

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

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

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

Re: kwidgetsaddons on OS X: menus become menu items

2017-02-07 Thread René J . V . Bertin 
On Tuesday February 7 2017 01:15:23 Marko Käning wrote:

Hi,

>BTW, has this problem disappeared in the meantime?

Not really. The Qt error messages are no longer printed, but the underlying 
cause is still there. QActions that are added to a Mac native menu are 
reparented and cannot also be used elsewhere. I know David Faure has picked up 
on that a while back, I don't know if he has managed to get any clarity on the 
why and how of this regression (w.r.t. Qt4). 

There is 1 workaround that I know of: avoid the native menubar in applications 
that rely too heavily on reusing QActions . This is why QtCurve now offers that 
choice as an option on Mac.

I have no idea if Kate's official Mac build implements another solution; 
there's nothing in the code that suggests it in any way.

R.

[Differential] [Closed] D4416: Send desktopfilename as part of notifyByPopup hints

2017-02-07 Thread David Edmundson 
This revision was automatically updated to reflect the committed changes.
Closed by commit R289:80c5aa01d207: Send desktopfilename as part of 
notifyByPopup hints (authored by davidedmundson).

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4416?vs=10866=10992

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

AFFECTED FILES
  src/notifybypopup.cpp

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

To: davidedmundson, #plasma, apol
Cc: graesslin, mck182, hein, plasma-devel, #frameworks, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

[Differential] [Request, 2 lines] D4471: Fix KCModule::setAuthAction error checking

2017-02-07 Thread David Edmundson 
davidedmundson created this revision.
davidedmundson added a reviewer: Plasma.
Restricted Application added projects: Plasma, Frameworks.
Restricted Application added subscribers: Frameworks, plasma-devel.

REVISION SUMMARY
  The current code checks if the existing action is valid rather the
  argument; and if it is invalid does an early return. That means you
  can't ever properly set a valid action.

TEST PLAN
  Started clock KCM which uses this, confirmed we got no warning

REPOSITORY
  R265 KConfigWidgets

BRANCH
  master

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

AFFECTED FILES
  src/kcmodule.cpp

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

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