[Differential] [Updated, 392 lines] D4425: Add support for flatpak portals

[Jan Grulich]

jgrulich updated this revision to Diff 10951.
jgrulich marked 11 inline comments as done.
jgrulich added a comment.


  - Fix mentioned issues

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4425?vs=10890=10951

BRANCH
  master

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

AFFECTED FILES
  src/CMakeLists.txt
  src/knotificationmanager.cpp
  src/notifybyflatpak.cpp
  src/notifybyflatpak.h

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

To: jgrulich, mck182
Cc: broulik, apol, #frameworks

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

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kio%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/419/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 06 Feb 2017 05:01:33 +
Build duration: 5 min 14 sec

CHANGE SET
Revision b877345b63c1237ec09f3dbf9ec74267224b23cb by scripty: (SVN_SILENT made 
messages (.desktop file) - always resolve ours)
  change: edit src/urifilters/ikws/searchproviders/msdn.desktop


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 29720/51615 
(58%)CONDITIONAL 16318/38747 (42%)

By packages
  
autotests
FILES 67/67 (100%)CLASSES 67/67 (100%)LINE 7915/8245 
(96%)CONDITIONAL 4427/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 8066/14179 
(57%)CONDITIONAL 4420/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 3449/7559 
(46%)CONDITIONAL 1281/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 1766/3780 
(47%)CONDITIONAL 1277/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 705/1139 
(62%)CONDITIONAL 402/833 (48%)
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 377/594 (63%)CONDITIONAL 
280/580 (48%)
src.kpasswdserver.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 283/286 (99%)CONDITIONAL 
144/256 (56%)
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 3632/11025 
(33%)CONDITIONAL 1746/7100 (25%)

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

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kio%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/419/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 06 Feb 2017 05:01:33 +
Build duration: 5 min 14 sec

CHANGE SET
Revision b877345b63c1237ec09f3dbf9ec74267224b23cb by scripty: (SVN_SILENT made 
messages (.desktop file) - always resolve ours)
  change: edit src/urifilters/ikws/searchproviders/msdn.desktop


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 29720/51615 
(58%)CONDITIONAL 16318/38747 (42%)

By packages
  
autotests
FILES 67/67 (100%)CLASSES 67/67 (100%)LINE 7915/8245 
(96%)CONDITIONAL 4427/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 8066/14179 
(57%)CONDITIONAL 4420/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 3449/7559 
(46%)CONDITIONAL 1281/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 1766/3780 
(47%)CONDITIONAL 1277/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 705/1139 
(62%)CONDITIONAL 402/833 (48%)
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 377/594 (63%)CONDITIONAL 
280/580 (48%)
src.kpasswdserver.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 283/286 (99%)CONDITIONAL 
144/256 (56%)
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 3632/11025 
(33%)CONDITIONAL 1746/7100 (25%)

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

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/kio%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/423/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 06 Feb 2017 05:01:34 +
Build duration: 7 min 28 sec

CHANGE SET
Revision b877345b63c1237ec09f3dbf9ec74267224b23cb by scripty: (SVN_SILENT made 
messages (.desktop file) - always resolve ours)
  change: edit src/urifilters/ikws/searchproviders/msdn.desktop


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 52 test(s), Skipped: 0 test(s), Total: 
53 test(s)Failed: TestSuite.kiocore-threadtest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 21/21 (100%)FILES 273/342 (80%)CLASSES 273/342 (80%)LINE 29707/51580 
(58%)CONDITIONAL 16309/38725 (42%)

By packages
  
autotests
FILES 66/66 (100%)CLASSES 66/66 (100%)LINE 7883/8210 
(96%)CONDITIONAL 4414/8642 (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 8077/14179 
(57%)CONDITIONAL 4419/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 3449/7559 
(46%)CONDITIONAL 1281/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 1766/3780 
(47%)CONDITIONAL 1277/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 705/1139 
(62%)CONDITIONAL 402/833 (48%)
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 377/594 (63%)CONDITIONAL 
280/580 (48%)
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: Review Request 129921: [kio-extras] thumbnails should be a clean image representation without "hard-coded" borders or design elements

[Diego S.]

> On Feb. 5, 2017, 9:50 a.m., David Faure wrote:
> > Can you also deprecate DrawFrame, then?

How does stuff get deprecated in Frameworks? I don't have a lot of kde 
development (or development in general) experience. I also don't have a proper 
dev environment setup atm.


- Diego


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


On Feb. 4, 2017, 9:51 p.m., Diego S. wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129921/
> ---
> 
> (Updated Feb. 4, 2017, 9:51 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio-extras
> 
> 
> Description
> ---
> 
> This patch is part of https://git.reviewboard.kde.org/r/129918/ - see the 
> discussion on the old patch https://git.reviewboard.kde.org/r/118961/.
> 
> Many thumbnail generators currently return ThumbCreator::DrawFrame in their 
> flags() function which causes a 90's style 1px "3d" border to be added to the 
> thumbnail image.
> This causes problems when consumers of these thumbnails (like Dolphin) expect 
> a clean image representation of the file, without hard-coded design elements.
> It also looks really bad when any resizing/scaling happens or when the 
> thumbnail is shown on a tooltip. Especially now that the UI has been 
> modernized with Breeze.
> 
> 
> Diffs
> -
> 
>   thumbnail/comiccreator.cpp d8f4473e1c7fa4595ad8defc406ad945d1ac38ac 
>   thumbnail/djvucreator.cpp 1547831bfaa875a14a59659a2bebc86a2cd7c024 
>   thumbnail/htmlcreator.cpp 29d1902794abfa3cf8ba2ab97648e1bc2d8e7a73 
>   thumbnail/kritacreator.cpp 3b24bc3d2fa7b22662c25da1421cbd542d7efed6 
>   thumbnail/textcreator.cpp 7c0263c8e8cb4f88d1429b5714c51c2939d817f0 
> 
> Diff: https://git.reviewboard.kde.org/r/129921/diff/
> 
> 
> Testing
> ---
> 
> Builds.
> 
> 
> Thanks,
> 
> Diego S.
> 
>

Jenkins-kde-ci: kio master kf5-qt5 » Linux,gcc - Build # 418 - Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/kio%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/418/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 06 Feb 2017 00:00:41 +
Build duration: 6 min 58 sec

CHANGE SET
No changes


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 52 test(s), Skipped: 0 test(s), Total: 
53 test(s)Failed: TestSuite.kiocore-threadtest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 21/21 (100%)FILES 273/342 (80%)CLASSES 273/342 (80%)LINE 29673/51580 
(58%)CONDITIONAL 16304/38725 (42%)

By packages
  
autotests
FILES 66/66 (100%)CLASSES 66/66 (100%)LINE 7883/8210 
(96%)CONDITIONAL 4416/8642 (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 8051/14179 
(57%)CONDITIONAL 4414/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 3449/7559 
(46%)CONDITIONAL 1281/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 1766/3780 
(47%)CONDITIONAL 1277/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 705/1139 
(62%)CONDITIONAL 402/833 (48%)
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 377/594 (63%)CONDITIONAL 
280/580 (48%)
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 3632/11025 
(33%)CONDITIONAL 1747/7100 (25%)

Jenkins-kde-ci: kservice master kf5-qt5 » Linux,gcc - Build # 241 - Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/kservice%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/241/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 05 Feb 2017 23:21:00 +
Build duration: 2 min 4 sec

CHANGE SET
No changes


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 10 test(s), Skipped: 0 test(s), Total: 
11 test(s)Failed: TestSuite.ksycocathreadtest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 6/7 (86%)FILES 75/84 (89%)CLASSES 75/84 (89%)LINE 5475/8008 
(68%)CONDITIONAL 2977/6178 (48%)

By packages
  
autotests
FILES 14/14 (100%)CLASSES 14/14 (100%)LINE 1448/1554 
(93%)CONDITIONAL 893/1792 (50%)
src.kbuildsycoca
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 61/67 (91%)CONDITIONAL 
15/20 (75%)
src.kdeinit
FILES 0/2 (0%)CLASSES 0/2 (0%)LINE 0/326 (0%)CONDITIONAL 0/262 
(0%)
src.plugin
FILES 2/3 (67%)CLASSES 2/3 (67%)LINE 47/100 (47%)CONDITIONAL 
36/96 (38%)
src.services
FILES 29/30 (97%)CLASSES 29/30 (97%)LINE 1766/3045 
(58%)CONDITIONAL 764/1892 (40%)
src.sycoca
FILES 26/31 (84%)CLASSES 26/31 (84%)LINE 2045/2796 
(73%)CONDITIONAL 1235/2066 (60%)
tests.pluginlocator
FILES 3/3 (100%)CLASSES 3/3 (100%)LINE 108/120 (90%)CONDITIONAL 
34/50 (68%)

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

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kservice%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/236/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 05 Feb 2017 23:06:56 +
Build duration: 1 min 39 sec

CHANGE SET
No changes


JUNIT RESULTS

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

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 6/7 (86%)FILES 75/84 (89%)CLASSES 75/84 (89%)LINE 5482/8008 
(68%)CONDITIONAL 2966/6178 (48%)

By packages
  
autotests
FILES 14/14 (100%)CLASSES 14/14 (100%)LINE 1464/1554 
(94%)CONDITIONAL 892/1792 (50%)
src.kbuildsycoca
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 61/67 (91%)CONDITIONAL 
15/20 (75%)
src.kdeinit
FILES 0/2 (0%)CLASSES 0/2 (0%)LINE 0/326 (0%)CONDITIONAL 0/262 
(0%)
src.plugin
FILES 2/3 (67%)CLASSES 2/3 (67%)LINE 47/100 (47%)CONDITIONAL 
36/96 (38%)
src.services
FILES 29/30 (97%)CLASSES 29/30 (97%)LINE 1765/3045 
(58%)CONDITIONAL 761/1892 (40%)
src.sycoca
FILES 26/31 (84%)CLASSES 26/31 (84%)LINE 2037/2796 
(73%)CONDITIONAL 1228/2066 (59%)
tests.pluginlocator
FILES 3/3 (100%)CLASSES 3/3 (100%)LINE 108/120 (90%)CONDITIONAL 
34/50 (68%)

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

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kservice%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/236/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 05 Feb 2017 23:06:56 +
Build duration: 1 min 39 sec

CHANGE SET
No changes


JUNIT RESULTS

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

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 6/7 (86%)FILES 75/84 (89%)CLASSES 75/84 (89%)LINE 5482/8008 
(68%)CONDITIONAL 2966/6178 (48%)

By packages
  
autotests
FILES 14/14 (100%)CLASSES 14/14 (100%)LINE 1464/1554 
(94%)CONDITIONAL 892/1792 (50%)
src.kbuildsycoca
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 61/67 (91%)CONDITIONAL 
15/20 (75%)
src.kdeinit
FILES 0/2 (0%)CLASSES 0/2 (0%)LINE 0/326 (0%)CONDITIONAL 0/262 
(0%)
src.plugin
FILES 2/3 (67%)CLASSES 2/3 (67%)LINE 47/100 (47%)CONDITIONAL 
36/96 (38%)
src.services
FILES 29/30 (97%)CLASSES 29/30 (97%)LINE 1765/3045 
(58%)CONDITIONAL 761/1892 (40%)
src.sycoca
FILES 26/31 (84%)CLASSES 26/31 (84%)LINE 2037/2796 
(73%)CONDITIONAL 1228/2066 (59%)
tests.pluginlocator
FILES 3/3 (100%)CLASSES 3/3 (100%)LINE 108/120 (90%)CONDITIONAL 
34/50 (68%)

Re: Phabricator differential is not good - WAS - Re: Phabricator: All repositories registered - upcoming workflow changes

[Albert Astals Cid]

El dilluns, 6 de febrer de 2017, a les 10:19:30 CET, Ben Cooksley va escriure:
> On Mon, Feb 6, 2017 at 8:39 AM, Albert Astals Cid  wrote:
> > El dilluns, 6 de febrer de 2017, a les 8:18:04 CET, Ben Cooksley va 
escriure:
> >> On Sun, Feb 5, 2017 at 6:24 AM, Albert Astals Cid  wrote:
> >> > El dissabte, 4 de febrer de 2017, a les 12:44:54 CET, Ben Cooksley va
> >> > 
> >> > escriure:
> >> >> On Sat, Feb 4, 2017 at 11:41 AM, Albert Astals Cid  
wrote:
> >> >> > El divendres, 3 de febrer de 2017, a les 21:06:08 CET, Ben Cooksley
> >> >> > va
> >> >> > 
> >> >> > escriure:
> >> >> >> On Fri, Feb 3, 2017 at 12:18 PM, Albert Astals Cid 
> > 
> > wrote:
> >> >> >> > El diumenge, 29 de gener de 2017, a les 8:32:21 CET, Ben Cooksley
> >> >> >> > va
> >> >> > 
> >> >> > escriure:
> >> >> >> >> Hi everyone,
> >> >> >> >> 
> >> >> >> >> We've just completed the registration of all mainline
> >> >> >> >> repositories
> >> >> >> >> (not including Websites or Sysadmin namespaced ones) on
> >> >> >> >> Phabricator.
> >> >> >> >> Thanks go to Luigi Toscano for providing significant assistance
> >> >> >> >> with
> >> >> >> >> this process.
> >> >> >> >> 
> >> >> >> >> From this point forward, communities should be moving away from
> >> >> >> >> Reviewboard to Phabricator for conducting code review.
> >> >> >> > 
> >> >> >> > I just created first patch with the phabricator web interface.
> >> >> >> > 
> >> >> >> > Found one minor and one major problem.
> >> >> >> > 
> >> >> >> > Minor problem:
> >> >> >> >  * You can't update the diff before creating a "Revision", so if
> >> >> >> >  you
> >> >> >> >  realize
> >> >> >> > 
> >> >> >> > your diff was wrong, back luck, you either leave the diff
> >> >> >> > floating
> >> >> >> > in
> >> >> >> > the
> >> >> >> > limbo or you create the Revision and the update the diff, showing
> >> >> >> > the
> >> >> >> > world
> >> >> >> > your mistake for no reason
> >> >> >> > https://phabricator.kde.org/D4422?vs=10881=10882
> >> >> >> 
> >> >> >> Interesting. It might be worth asking upstream about that.
> >> >> >> 
> >> >> >> > Major problem:
> >> >> >> >  * It doesn't show context
> >> >> >> > 
> >> >> >> > https://phabricator.kde.org/D4422
> >> >> >> > 
> >> >> >> > "Context not available." is terrible, how is one supposed to
> >> >> >> > review
> >> >> >> > without
> >> >> >> > being able to read the rest of the code?
> >> >> >> > 
> >> >> >> > This is a deal breaker for me.
> >> >> >> 
> >> >> >> Please see https://secure.phabricator.com/T5029
> >> >> > 
> >> >> > As said on IRC, the fact that this has been open for almost 3 years
> >> >> > is
> >> >> > more a concern than a relief.
> >> >> 
> >> >> I've inquired with upstream, and they've indicated that at the moment
> >> >> T5029 isn't on their roadmap for implementation (although T5000 and
> >> >> T182 are).
> >> >> 
> >> >> Their target audience is primarily corporate development workflows,
> >> >> for which requiring use of Arcanist isn't an issue.
> >> >> 
> >> >> >> This only occurs when patches are uploaded from the web interface
> >> >> >> and
> >> >> >> the patch in question has minimal context.
> >> >> >> At this time Phabricator is not able to automatically resolve
> >> >> >> context
> >> >> >> using markers in the patch (there are certain complexities involved
> >> >> >> for some SCMs, particularly for SVN - which Phabricator supports)
> >> >> >> 
> >> >> >> The fix for this is to either:
> >> >> >> a) Use Arcanist, the recommended tool for working with Phabricator
> >> >> >> (this is no different to rb-tools for Reviewboard)
> >> >> > 
> >> >> > This is not ok, the web interface for reviewboard was as good as
> >> >> > rb-tools
> >> >> > (i guess tbh i never used them) and "forcing" the use of a weird
> >> >> > tool
> >> >> > noone has heard of is not a good way to attract new contributors
> >> >> 
> >> >> New contributors who aren't willing to install Arcanist can use diff
> >> >> -U99 I would imagine?
> >> > 
> >> > Yes, If there's an easy way for them to know they should (which afaics
> >> > is
> >> > not right now).
> >> 
> >> Okay, we'll look into adding a message box or something there
> >> explaining the need to use diff -U99.
> > 
> > FWIW i thought that diff -U99 would give you the same behaviour that when
> > using arc (somehow i thought phabricator was not very smart and needed
> > more
> > lines to actually match the patch against the whole file).
> > 
> > But no, using diff -U99 only gives you more lines because you used -U99,
> > but you can't still expand the whole file like when using arc.
> 
> Are you essentially saying that the migration cannot proceed, because
> Phabricator doesn't know how to expand patches?

I'm saying i personally think it's a severe regression and i will not be using 
Differential for the very few patches i create until i'm forced to, since 
Reviewboard is much more useful for my worflow.

Obviously, given how few 

[Differential] [Commented On] D4439: KDirWatch: fix memory leak on destruction.

[Albert Astals Cid]

aacid added a comment.


  In https://phabricator.kde.org/D4439#83326, @dfaure wrote:
  
  > In https://phabricator.kde.org/D4439#83310, @aacid wrote:
  >
  > > In https://phabricator.kde.org/D4439#83166, @dfaure wrote:
  > >
  > > > It's not crazy, but
  > > >
  > > > - then it should use QVector instead of QList (Client is a "big" 
struct, bigger than a pointer)
  > >
  > >
  > > The problem with QVector is that it doesn't have erase(iterator)  built 
in like QList has.
  >
  >
  > I see it in the docu:
  >
  > iterator QVector::erase(iterator pos)
  >  Removes the item pointed to by the iterator pos from the vector, and 
returns an iterator to the next item in the vector (which may be end()).
  >
  > >> - I would be worried about copies happening unexpectedly (can this code 
compile with forbidden copy ctor for Client? I guess not as is due to insertion 
into the vector but maybe std::move can be used there, or simply setting 
the members directly onto a ref for vector[i]).
  > > 
  > > Without the copy constructor there's quite a lot of things that don't 
work. OTOH all the data in Client is basiclaly POD, but i guess at some point 
it could be "a lot of copying", if you think it's worth it i can investigate 
some "less Q and more C++11-y stuff" and see if std::move or something works
  >
  > If you fancy looking into it, go ahead. Otherwise tell me and I will.
  
  
  You take it, had a look and it's not as easy as i'd like, m_mapEntries makes 
it more difficult, and also the code kind of confuses me by m_entries 
containing pointers that seem to be extracted from the map itself? Maybe let's 
not touch this and just go with your simpler solution :D

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

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

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

To: dfaure, mpyne, aacid
Cc: markg, #frameworks

Re: Phabricator differential is not good - WAS - Re: Phabricator: All repositories registered - upcoming workflow changes

[Ben Cooksley]

On Mon, Feb 6, 2017 at 8:39 AM, Albert Astals Cid  wrote:
> El dilluns, 6 de febrer de 2017, a les 8:18:04 CET, Ben Cooksley va escriure:
>> On Sun, Feb 5, 2017 at 6:24 AM, Albert Astals Cid  wrote:
>> > El dissabte, 4 de febrer de 2017, a les 12:44:54 CET, Ben Cooksley va
>> >
>> > escriure:
>> >> On Sat, Feb 4, 2017 at 11:41 AM, Albert Astals Cid  wrote:
>> >> > El divendres, 3 de febrer de 2017, a les 21:06:08 CET, Ben Cooksley va
>> >> >
>> >> > escriure:
>> >> >> On Fri, Feb 3, 2017 at 12:18 PM, Albert Astals Cid 
> wrote:
>> >> >> > El diumenge, 29 de gener de 2017, a les 8:32:21 CET, Ben Cooksley va
>> >> >
>> >> > escriure:
>> >> >> >> Hi everyone,
>> >> >> >>
>> >> >> >> We've just completed the registration of all mainline repositories
>> >> >> >> (not including Websites or Sysadmin namespaced ones) on
>> >> >> >> Phabricator.
>> >> >> >> Thanks go to Luigi Toscano for providing significant assistance
>> >> >> >> with
>> >> >> >> this process.
>> >> >> >>
>> >> >> >> From this point forward, communities should be moving away from
>> >> >> >> Reviewboard to Phabricator for conducting code review.
>> >> >> >
>> >> >> > I just created first patch with the phabricator web interface.
>> >> >> >
>> >> >> > Found one minor and one major problem.
>> >> >> >
>> >> >> > Minor problem:
>> >> >> >  * You can't update the diff before creating a "Revision", so if you
>> >> >> >  realize
>> >> >> >
>> >> >> > your diff was wrong, back luck, you either leave the diff floating
>> >> >> > in
>> >> >> > the
>> >> >> > limbo or you create the Revision and the update the diff, showing
>> >> >> > the
>> >> >> > world
>> >> >> > your mistake for no reason
>> >> >> > https://phabricator.kde.org/D4422?vs=10881=10882
>> >> >>
>> >> >> Interesting. It might be worth asking upstream about that.
>> >> >>
>> >> >> > Major problem:
>> >> >> >  * It doesn't show context
>> >> >> >
>> >> >> > https://phabricator.kde.org/D4422
>> >> >> >
>> >> >> > "Context not available." is terrible, how is one supposed to review
>> >> >> > without
>> >> >> > being able to read the rest of the code?
>> >> >> >
>> >> >> > This is a deal breaker for me.
>> >> >>
>> >> >> Please see https://secure.phabricator.com/T5029
>> >> >
>> >> > As said on IRC, the fact that this has been open for almost 3 years is
>> >> > more a concern than a relief.
>> >>
>> >> I've inquired with upstream, and they've indicated that at the moment
>> >> T5029 isn't on their roadmap for implementation (although T5000 and
>> >> T182 are).
>> >>
>> >> Their target audience is primarily corporate development workflows,
>> >> for which requiring use of Arcanist isn't an issue.
>> >>
>> >> >> This only occurs when patches are uploaded from the web interface and
>> >> >> the patch in question has minimal context.
>> >> >> At this time Phabricator is not able to automatically resolve context
>> >> >> using markers in the patch (there are certain complexities involved
>> >> >> for some SCMs, particularly for SVN - which Phabricator supports)
>> >> >>
>> >> >> The fix for this is to either:
>> >> >> a) Use Arcanist, the recommended tool for working with Phabricator
>> >> >> (this is no different to rb-tools for Reviewboard)
>> >> >
>> >> > This is not ok, the web interface for reviewboard was as good as
>> >> > rb-tools
>> >> > (i guess tbh i never used them) and "forcing" the use of a weird tool
>> >> > noone has heard of is not a good way to attract new contributors
>> >>
>> >> New contributors who aren't willing to install Arcanist can use diff
>> >> -U99 I would imagine?
>> >
>> > Yes, If there's an easy way for them to know they should (which afaics is
>> > not right now).
>>
>> Okay, we'll look into adding a message box or something there
>> explaining the need to use diff -U99.
>
> FWIW i thought that diff -U99 would give you the same behaviour that when
> using arc (somehow i thought phabricator was not very smart and needed more
> lines to actually match the patch against the whole file).
>
> But no, using diff -U99 only gives you more lines because you used -U99, but
> you can't still expand the whole file like when using arc.

Are you essentially saying that the migration cannot proceed, because
Phabricator doesn't know how to expand patches?

I'd suggest using "diff -U9" or some other massively
long number, if having the full file available is a hard requirement.

>
> Cheers,
>   Albert

Regards,
Ben

>
>>
>> > Cheers,
>> >
>> >   Albert
>>
>> Regards,
>> Ben
>>
>> >> > Cheers,
>> >> >
>> >> >   Albert
>> >>
>> >> Regards,
>> >> Ben
>
>

[Differential] [Commented On] D4439: KDirWatch: fix memory leak on destruction.

[David Faure]

dfaure added a comment.


  In https://phabricator.kde.org/D4439#83310, @aacid wrote:
  
  > In https://phabricator.kde.org/D4439#83166, @dfaure wrote:
  >
  > > It's not crazy, but
  > >
  > > - then it should use QVector instead of QList (Client is a "big" struct, 
bigger than a pointer)
  >
  >
  > The problem with QVector is that it doesn't have erase(iterator)  built in 
like QList has.
  
  
  I see it in the docu:
  
  iterator QVector::erase(iterator pos)
  Removes the item pointed to by the iterator pos from the vector, and returns 
an iterator to the next item in the vector (which may be end()).
  
  >> - I would be worried about copies happening unexpectedly (can this code 
compile with forbidden copy ctor for Client? I guess not as is due to insertion 
into the vector but maybe std::move can be used there, or simply setting 
the members directly onto a ref for vector[i]).
  > 
  > Without the copy constructor there's quite a lot of things that don't work. 
OTOH all the data in Client is basiclaly POD, but i guess at some point it 
could be "a lot of copying", if you think it's worth it i can investigate some 
"less Q and more C++11-y stuff" and see if std::move or something works
  
  If you fancy looking into it, go ahead. Otherwise tell me and I will.

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

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

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

To: dfaure, mpyne, aacid
Cc: markg, #frameworks

[Differential] [Commented On] D4439: KDirWatch: fix memory leak on destruction.

[Mark Gaiser]

markg added a comment.


  In https://phabricator.kde.org/D4439#83310, @aacid wrote:
  
  > In https://phabricator.kde.org/D4439#83166, @dfaure wrote:
  >
  > > It's not crazy, but
  > >
  > > - then it should use QVector instead of QList (Client is a "big" struct, 
bigger than a pointer)
  >
  >
  > The problem with QVector is that it doesn't have erase(iterator)  built in 
like QList has.
  >
  > > - I would be worried about copies happening unexpectedly (can this code 
compile with forbidden copy ctor for Client? I guess not as is due to insertion 
into the vector but maybe std::move can be used there, or simply setting 
the members directly onto a ref for vector[i]).
  >
  > Without the copy constructor there's quite a lot of things that don't work. 
OTOH all the data in Client is basiclaly POD, but i guess at some point it 
could be "a lot of copying", if you think it's worth it i can investigate some 
"less Q and more C++11-y stuff" and see if std::move or something works
  
  
  std::move and std::vector will work :)
  You "just" have to implement the move semantics for the Client class.
  Follow this: http://en.cppreference.com/w/cpp/language/move_assignment
  Look closely at the example and in there at "struct A" at these two snippets: 
(first one is the move constructor, second one is the move assignment operator)
  
A(A&& o) : s(std::move(o.s)) { }
  
  
  
A& operator=(A&& other)
  
  Also, don't forget to explicitly delete the copy operations (in the public 
section of your class):
  A(A const &) = delete;
  void operator=(A const ) = delete;
  
  Good luck :)

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

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

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

To: dfaure, mpyne, aacid
Cc: markg, #frameworks

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

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kfilemetadata%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/164/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 05 Feb 2017 19:48:30 +
Build duration: 7 min 54 sec

CHANGE SET
Revision 17aaf4ec09a5bf2e3c213cdfbcd3938c6c81c2cd by matthieu_gallien: (fix 
build of kfilemetadata with taglib version 1.9)
  change: edit src/extractors/taglibextractor.cpp


JUNIT RESULTS

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

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 4/4 (100%)FILES 45/56 (80%)CLASSES 45/56 (80%)LINE 1487/2114 
(70%)CONDITIONAL 2409/5421 (44%)

By packages
  
autotests
FILES 20/20 (100%)CLASSES 20/20 (100%)LINE 327/327 
(100%)CONDITIONAL 1066/2124 (50%)
src
FILES 11/22 (50%)CLASSES 11/22 (50%)LINE 587/1022 
(57%)CONDITIONAL 319/903 (35%)
src.extractors
FILES 12/12 (100%)CLASSES 12/12 (100%)LINE 550/733 
(75%)CONDITIONAL 990/2310 (43%)
src.writers
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 23/32 (72%)CONDITIONAL 
34/84 (40%)

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

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kfilemetadata%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/164/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 05 Feb 2017 19:48:30 +
Build duration: 7 min 54 sec

CHANGE SET
Revision 17aaf4ec09a5bf2e3c213cdfbcd3938c6c81c2cd by matthieu_gallien: (fix 
build of kfilemetadata with taglib version 1.9)
  change: edit src/extractors/taglibextractor.cpp


JUNIT RESULTS

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

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 4/4 (100%)FILES 45/56 (80%)CLASSES 45/56 (80%)LINE 1487/2114 
(70%)CONDITIONAL 2409/5421 (44%)

By packages
  
autotests
FILES 20/20 (100%)CLASSES 20/20 (100%)LINE 327/327 
(100%)CONDITIONAL 1066/2124 (50%)
src
FILES 11/22 (50%)CLASSES 11/22 (50%)LINE 587/1022 
(57%)CONDITIONAL 319/903 (35%)
src.extractors
FILES 12/12 (100%)CLASSES 12/12 (100%)LINE 550/733 
(75%)CONDITIONAL 990/2310 (43%)
src.writers
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 23/32 (72%)CONDITIONAL 
34/84 (40%)

Jenkins-kde-ci: kfilemetadata master kf5-qt5 » Linux,gcc - Build # 166 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kfilemetadata%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/166/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 05 Feb 2017 19:48:30 +
Build duration: 1 min 58 sec

CHANGE SET
Revision 17aaf4ec09a5bf2e3c213cdfbcd3938c6c81c2cd by matthieu_gallien: (fix 
build of kfilemetadata with taglib version 1.9)
  change: edit src/extractors/taglibextractor.cpp


JUNIT RESULTS

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

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 4/4 (100%)FILES 49/60 (82%)CLASSES 49/60 (82%)LINE 1547/ 
(70%)CONDITIONAL 2534/5740 (44%)

By packages
  
autotests
FILES 22/22 (100%)CLASSES 22/22 (100%)LINE 353/353 
(100%)CONDITIONAL 1150/2292 (50%)
src
FILES 11/22 (50%)CLASSES 11/22 (50%)LINE 587/1022 
(57%)CONDITIONAL 315/896 (35%)
src.extractors
FILES 14/14 (100%)CLASSES 14/14 (100%)LINE 584/815 
(72%)CONDITIONAL 1035/2468 (42%)
src.writers
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 23/32 (72%)CONDITIONAL 
34/84 (40%)

Jenkins-kde-ci: kfilemetadata master kf5-qt5 » Linux,gcc - Build # 166 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kfilemetadata%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/166/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 05 Feb 2017 19:48:30 +
Build duration: 1 min 58 sec

CHANGE SET
Revision 17aaf4ec09a5bf2e3c213cdfbcd3938c6c81c2cd by matthieu_gallien: (fix 
build of kfilemetadata with taglib version 1.9)
  change: edit src/extractors/taglibextractor.cpp


JUNIT RESULTS

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

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 4/4 (100%)FILES 49/60 (82%)CLASSES 49/60 (82%)LINE 1547/ 
(70%)CONDITIONAL 2534/5740 (44%)

By packages
  
autotests
FILES 22/22 (100%)CLASSES 22/22 (100%)LINE 353/353 
(100%)CONDITIONAL 1150/2292 (50%)
src
FILES 11/22 (50%)CLASSES 11/22 (50%)LINE 587/1022 
(57%)CONDITIONAL 315/896 (35%)
src.extractors
FILES 14/14 (100%)CLASSES 14/14 (100%)LINE 584/815 
(72%)CONDITIONAL 1035/2468 (42%)
src.writers
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 23/32 (72%)CONDITIONAL 
34/84 (40%)

Re: Review Request 129925: fix build of kfilemetadata with taglib version 1.9

[Matthieu Gallien]

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

(Updated Feb. 5, 2017, 7:48 p.m.)


Status
--

This change has been marked as submitted.


Review request for Baloo and KDE Frameworks.


Changes
---

Submitted with commit 17aaf4ec09a5bf2e3c213cdfbcd3938c6c81c2cd by Matthieu 
Gallien to branch master.


Repository: kfilemetadata


Description
---

fix build of kfilemetadata with taglib version 1.9


Diffs
-

  src/extractors/taglibextractor.cpp 99c66f4593d08a71462527c89c2c084b8c638ad6 

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


Testing
---

Automatic tests are OK.

I have checked from taglib github repository that the API exists in 1.9 version.


Thanks,

Matthieu Gallien

Re: Phabricator differential is not good - WAS - Re: Phabricator: All repositories registered - upcoming workflow changes

[Albert Astals Cid]

El dilluns, 6 de febrer de 2017, a les 8:18:04 CET, Ben Cooksley va escriure:
> On Sun, Feb 5, 2017 at 6:24 AM, Albert Astals Cid  wrote:
> > El dissabte, 4 de febrer de 2017, a les 12:44:54 CET, Ben Cooksley va
> > 
> > escriure:
> >> On Sat, Feb 4, 2017 at 11:41 AM, Albert Astals Cid  wrote:
> >> > El divendres, 3 de febrer de 2017, a les 21:06:08 CET, Ben Cooksley va
> >> > 
> >> > escriure:
> >> >> On Fri, Feb 3, 2017 at 12:18 PM, Albert Astals Cid  
wrote:
> >> >> > El diumenge, 29 de gener de 2017, a les 8:32:21 CET, Ben Cooksley va
> >> > 
> >> > escriure:
> >> >> >> Hi everyone,
> >> >> >> 
> >> >> >> We've just completed the registration of all mainline repositories
> >> >> >> (not including Websites or Sysadmin namespaced ones) on
> >> >> >> Phabricator.
> >> >> >> Thanks go to Luigi Toscano for providing significant assistance
> >> >> >> with
> >> >> >> this process.
> >> >> >> 
> >> >> >> From this point forward, communities should be moving away from
> >> >> >> Reviewboard to Phabricator for conducting code review.
> >> >> > 
> >> >> > I just created first patch with the phabricator web interface.
> >> >> > 
> >> >> > Found one minor and one major problem.
> >> >> > 
> >> >> > Minor problem:
> >> >> >  * You can't update the diff before creating a "Revision", so if you
> >> >> >  realize
> >> >> > 
> >> >> > your diff was wrong, back luck, you either leave the diff floating
> >> >> > in
> >> >> > the
> >> >> > limbo or you create the Revision and the update the diff, showing
> >> >> > the
> >> >> > world
> >> >> > your mistake for no reason
> >> >> > https://phabricator.kde.org/D4422?vs=10881=10882
> >> >> 
> >> >> Interesting. It might be worth asking upstream about that.
> >> >> 
> >> >> > Major problem:
> >> >> >  * It doesn't show context
> >> >> > 
> >> >> > https://phabricator.kde.org/D4422
> >> >> > 
> >> >> > "Context not available." is terrible, how is one supposed to review
> >> >> > without
> >> >> > being able to read the rest of the code?
> >> >> > 
> >> >> > This is a deal breaker for me.
> >> >> 
> >> >> Please see https://secure.phabricator.com/T5029
> >> > 
> >> > As said on IRC, the fact that this has been open for almost 3 years is
> >> > more a concern than a relief.
> >> 
> >> I've inquired with upstream, and they've indicated that at the moment
> >> T5029 isn't on their roadmap for implementation (although T5000 and
> >> T182 are).
> >> 
> >> Their target audience is primarily corporate development workflows,
> >> for which requiring use of Arcanist isn't an issue.
> >> 
> >> >> This only occurs when patches are uploaded from the web interface and
> >> >> the patch in question has minimal context.
> >> >> At this time Phabricator is not able to automatically resolve context
> >> >> using markers in the patch (there are certain complexities involved
> >> >> for some SCMs, particularly for SVN - which Phabricator supports)
> >> >> 
> >> >> The fix for this is to either:
> >> >> a) Use Arcanist, the recommended tool for working with Phabricator
> >> >> (this is no different to rb-tools for Reviewboard)
> >> > 
> >> > This is not ok, the web interface for reviewboard was as good as
> >> > rb-tools
> >> > (i guess tbh i never used them) and "forcing" the use of a weird tool
> >> > noone has heard of is not a good way to attract new contributors
> >> 
> >> New contributors who aren't willing to install Arcanist can use diff
> >> -U99 I would imagine?
> > 
> > Yes, If there's an easy way for them to know they should (which afaics is
> > not right now).
> 
> Okay, we'll look into adding a message box or something there
> explaining the need to use diff -U99.

FWIW i thought that diff -U99 would give you the same behaviour that when 
using arc (somehow i thought phabricator was not very smart and needed more 
lines to actually match the patch against the whole file).

But no, using diff -U99 only gives you more lines because you used -U99, but 
you can't still expand the whole file like when using arc.

Cheers,
  Albert

> 
> > Cheers,
> > 
> >   Albert
> 
> Regards,
> Ben
> 
> >> > Cheers,
> >> > 
> >> >   Albert
> >> 
> >> Regards,
> >> Ben

Re: Review Request 129925: fix build of kfilemetadata with taglib version 1.9

[David Faure]

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


Ship it!




Ship It!

- David Faure


On Feb. 5, 2017, 4:39 p.m., Matthieu Gallien wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129925/
> ---
> 
> (Updated Feb. 5, 2017, 4:39 p.m.)
> 
> 
> Review request for Baloo and KDE Frameworks.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> fix build of kfilemetadata with taglib version 1.9
> 
> 
> Diffs
> -
> 
>   src/extractors/taglibextractor.cpp 99c66f4593d08a71462527c89c2c084b8c638ad6 
> 
> Diff: https://git.reviewboard.kde.org/r/129925/diff/
> 
> 
> Testing
> ---
> 
> Automatic tests are OK.
> 
> I have checked from taglib github repository that the API exists in 1.9 
> version.
> 
> 
> Thanks,
> 
> Matthieu Gallien
> 
>

[Differential] [Commented On] D4439: KDirWatch: fix memory leak on destruction.

[Albert Astals Cid]

aacid added a comment.


  In https://phabricator.kde.org/D4439#83166, @dfaure wrote:
  
  > It's not crazy, but
  >
  > - then it should use QVector instead of QList (Client is a "big" struct, 
bigger than a pointer)
  
  
  The problem with QVector is that it doesn't have erase(iterator)  built in 
like QList has.
  
  > - I would be worried about copies happening unexpectedly (can this code 
compile with forbidden copy ctor for Client? I guess not as is due to insertion 
into the vector but maybe std::move can be used there, or simply setting 
the members directly onto a ref for vector[i]).
  
  Without the copy constructor there's quite a lot of things that don't work. 
OTOH all the data in Client is basiclaly POD, but i guess at some point it 
could be "a lot of copying", if you think it's worth it i can investigate some 
"less Q and more C++11-y stuff" and see if std::move or something works

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

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

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

To: dfaure, mpyne, aacid
Cc: markg, #frameworks

Re: Phabricator differential is not good - WAS - Re: Phabricator: All repositories registered - upcoming workflow changes

[Ben Cooksley]

On Sun, Feb 5, 2017 at 6:24 AM, Albert Astals Cid  wrote:
> El dissabte, 4 de febrer de 2017, a les 12:44:54 CET, Ben Cooksley va
> escriure:
>> On Sat, Feb 4, 2017 at 11:41 AM, Albert Astals Cid  wrote:
>> > El divendres, 3 de febrer de 2017, a les 21:06:08 CET, Ben Cooksley va
>> >
>> > escriure:
>> >> On Fri, Feb 3, 2017 at 12:18 PM, Albert Astals Cid  wrote:
>> >> > El diumenge, 29 de gener de 2017, a les 8:32:21 CET, Ben Cooksley va
>> >
>> > escriure:
>> >> >> Hi everyone,
>> >> >>
>> >> >> We've just completed the registration of all mainline repositories
>> >> >> (not including Websites or Sysadmin namespaced ones) on Phabricator.
>> >> >> Thanks go to Luigi Toscano for providing significant assistance with
>> >> >> this process.
>> >> >>
>> >> >> From this point forward, communities should be moving away from
>> >> >> Reviewboard to Phabricator for conducting code review.
>> >> >
>> >> > I just created first patch with the phabricator web interface.
>> >> >
>> >> > Found one minor and one major problem.
>> >> >
>> >> > Minor problem:
>> >> >  * You can't update the diff before creating a "Revision", so if you
>> >> >  realize
>> >> >
>> >> > your diff was wrong, back luck, you either leave the diff floating in
>> >> > the
>> >> > limbo or you create the Revision and the update the diff, showing the
>> >> > world
>> >> > your mistake for no reason
>> >> > https://phabricator.kde.org/D4422?vs=10881=10882
>> >>
>> >> Interesting. It might be worth asking upstream about that.
>> >>
>> >> > Major problem:
>> >> >  * It doesn't show context
>> >> >
>> >> > https://phabricator.kde.org/D4422
>> >> >
>> >> > "Context not available." is terrible, how is one supposed to review
>> >> > without
>> >> > being able to read the rest of the code?
>> >> >
>> >> > This is a deal breaker for me.
>> >>
>> >> Please see https://secure.phabricator.com/T5029
>> >
>> > As said on IRC, the fact that this has been open for almost 3 years is
>> > more a concern than a relief.
>>
>> I've inquired with upstream, and they've indicated that at the moment
>> T5029 isn't on their roadmap for implementation (although T5000 and
>> T182 are).
>>
>> Their target audience is primarily corporate development workflows,
>> for which requiring use of Arcanist isn't an issue.
>>
>> >> This only occurs when patches are uploaded from the web interface and
>> >> the patch in question has minimal context.
>> >> At this time Phabricator is not able to automatically resolve context
>> >> using markers in the patch (there are certain complexities involved
>> >> for some SCMs, particularly for SVN - which Phabricator supports)
>> >>
>> >> The fix for this is to either:
>> >> a) Use Arcanist, the recommended tool for working with Phabricator
>> >> (this is no different to rb-tools for Reviewboard)
>> >
>> > This is not ok, the web interface for reviewboard was as good as rb-tools
>> > (i guess tbh i never used them) and "forcing" the use of a weird tool
>> > noone has heard of is not a good way to attract new contributors
>>
>> New contributors who aren't willing to install Arcanist can use diff
>> -U99 I would imagine?
>
> Yes, If there's an easy way for them to know they should (which afaics is not
> right now).

Okay, we'll look into adding a message box or something there
explaining the need to use diff -U99.

>
> Cheers,
>   Albert

Regards,
Ben

>
>>
>> > Cheers,
>> >
>> >   Albert
>>
>> Regards,
>> Ben
>
>

Jenkins-kde-ci: kfilemetadata master kf5-qt5 » Linux,gcc - Build # 165 - Still Failing!

[no-reply]

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/kfilemetadata%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/165/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 05 Feb 2017 17:17:02 +
Build duration: 6 min 54 sec

CHANGE SET
Revision b3e71d1d7ad75f90864d2a626da860c60595ea4d by David Faure: (New 
maintainer for kfilemetadata)
  change: edit metainfo.yaml

Jenkins-kde-ci: kfilemetadata master stable-kf5-qt5 » Linux,gcc - Build # 163 - Still Failing!

[no-reply]

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/kfilemetadata%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/163/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 05 Feb 2017 17:17:01 +
Build duration: 6 min 6 sec

CHANGE SET
Revision b3e71d1d7ad75f90864d2a626da860c60595ea4d by David Faure: (New 
maintainer for kfilemetadata)
  change: edit metainfo.yaml

Re: Review Request 129925: fix build of kfilemetadata with taglib version 1.9

[Matthieu Gallien]

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

(Updated Feb. 5, 2017, 5:39 p.m.)


Review request for Baloo and KDE Frameworks.


Repository: kfilemetadata


Description
---

fix build of kfilemetadata with taglib version 1.9


Diffs
-

  src/extractors/taglibextractor.cpp 99c66f4593d08a71462527c89c2c084b8c638ad6 

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


Testing
---

Automatic tests are OK.

I have checked from taglib github repository that the API exists in 1.9 version.


Thanks,

Matthieu Gallien

Jenkins-kde-ci: kfilemetadata master kf5-qt5 » Linux,gcc - Build # 164 - Still Failing!

[no-reply]

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/kfilemetadata%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/164/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 05 Feb 2017 16:06:51 +
Build duration: 33 sec

CHANGE SET
Revision 7c7e985a4678fef5f5d0dd8faa9b9cb42e3844b4 by bvbfan: 
([ExtractorCollection] Use mimetype inheritance to return plugins)
  change: edit src/extractorcollection.cpp

[Differential] [Request, 177 lines] D4448: Don't use tier3 frameworks in unit tests

[Sune Vuorela]

svuorela created this revision.
svuorela added reviewers: sitter, vonreth, kde-frameworks-devel.
svuorela set the repository for this revision to R266 Breeze Icons.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  Port away from tier2 and 3 frameworks in the unit tests

TEST PLAN
  Tests still passes

REPOSITORY
  R266 Breeze Icons

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

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

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

To: svuorela, sitter, vonreth, kde-frameworks-devel
Cc: #frameworks

Re: Review Request 129720: [ExtractorCollection] Use mimetype inheritance to return plugins

[Anthony Fieroni]

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

(Updated Feb. 5, 2017, 4:06 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit 7c7e985a4678fef5f5d0dd8faa9b9cb42e3844b4 by Anthony 
Fieroni to branch master.


Repository: kfilemetadata


Description
---

startsWith is a weak precondition + guard for duplicate plugin


Diffs
-

  src/extractorcollection.cpp d0fbbea 

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


Testing
---


Thanks,

Anthony Fieroni

[Differential] [Closed] D4329: KMessageWidget: fix behaviour on overlapping calls of animatedShow/animatedHide

[Friedrich W. H. Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R236:ec02ee4b85a4: KMessageWidget: fix behaviour on 
overlapping calls of animatedShow/animatedHide (authored by kossebau).

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4329?vs=10838=10941

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

AFFECTED FILES
  autotests/kmessagewidgetautotest.cpp
  autotests/kmessagewidgetautotest.h
  src/kmessagewidget.cpp

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

To: kossebau, #frameworks, dhaumann
Cc: cfeck

Jenkins-kde-ci: kfilemetadata master stable-kf5-qt5 » Linux,gcc - Build # 161 - Failure!

[no-reply]

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/kfilemetadata%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/161/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 05 Feb 2017 15:30:26 +
Build duration: 1 min 53 sec

CHANGE SET
Revision 35ea63920a0398aa859b2944770fd9036c271166 by matthieu_gallien: (add 
more audio format to automatic test and fix problems)
  change: edit src/extractors/taglibextractor.cpp
  change: add autotests/samplefiles/test.flac
  change: add autotests/samplefiles/test.mp3
  change: add autotests/samplefiles/test.mpc
  change: add autotests/samplefiles/test.m4a
  change: edit autotests/taglibextractortest.cpp
  change: add autotests/samplefiles/test.ogg
Revision 6e64944bdbdad7a7d6e4262b390c8134c099fc46 by matthieu_gallien: (add a 
new property DiscNumber for audio files from multi-disc albums)
  change: edit src/properties.h
  change: edit src/extractors/taglibextractor.cpp
  change: edit src/propertyinfo.cpp
  change: edit autotests/taglibextractortest.cpp

Jenkins-kde-ci: kfilemetadata master kf5-qt5 » Linux,gcc - Build # 163 - Failure!

[no-reply]

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/kfilemetadata%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/163/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 05 Feb 2017 15:30:26 +
Build duration: 35 sec

CHANGE SET
Revision 35ea63920a0398aa859b2944770fd9036c271166 by matthieu_gallien: (add 
more audio format to automatic test and fix problems)
  change: add autotests/samplefiles/test.mpc
  change: edit src/extractors/taglibextractor.cpp
  change: add autotests/samplefiles/test.ogg
  change: add autotests/samplefiles/test.flac
  change: edit autotests/taglibextractortest.cpp
  change: add autotests/samplefiles/test.mp3
  change: add autotests/samplefiles/test.m4a
Revision 6e64944bdbdad7a7d6e4262b390c8134c099fc46 by matthieu_gallien: (add a 
new property DiscNumber for audio files from multi-disc albums)
  change: edit src/propertyinfo.cpp
  change: edit src/extractors/taglibextractor.cpp
  change: edit src/properties.h
  change: edit autotests/taglibextractortest.cpp

Re: Review Request 129798: add more audio formats to automatic test and fix one problem with Musepack

[Matthieu Gallien]

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

(Updated Feb. 5, 2017, 4:30 p.m.)


Status
--

This change has been marked as submitted.


Review request for Baloo and KDE Frameworks.


Changes
---

Submitted with commit 35ea63920a0398aa859b2944770fd9036c271166 by Matthieu 
Gallien to branch master.


Repository: kfilemetadata


Description
---

Musepack extraction of album artist was using the wrong tag name (tested
against easytag and picard).

See also this page (http://taglib.org/api/classTagLib_1_1APE_1_1Tag.html) that 
seems to indicate that APE is using "ALBUM ARTIST" with a space between both 
words.


Diffs
-

  autotests/samplefiles/test.flac PRE-CREATION 
  autotests/samplefiles/test.m4a PRE-CREATION 
  autotests/samplefiles/test.mp3 PRE-CREATION 
  autotests/samplefiles/test.mpc PRE-CREATION 
  autotests/samplefiles/test.ogg PRE-CREATION 
  autotests/taglibextractortest.cpp d46e49e 
  src/extractors/taglibextractor.cpp 8fcad93 

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


Testing
---

I have tested with automatic tests of KFileMetaData and also the encoding 
problem in Musepack format against Easytag and Picard applications.


Thanks,

Matthieu Gallien

[Differential] [Closed] D4444: Make tier3 dependency for tests optional

[Hannah von Reth]

This revision was automatically updated to reflect the committed changes.
Closed by commit R266:7109c9f7e5e6: Make tier3 dependency for tests optional 
(authored by vonreth).

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D?vs=10932=10935

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

AFFECTED FILES
  CMakeLists.txt
  autotests/CMakeLists.txt

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

To: vonreth, sitter, svuorela
Cc: svuorela, #frameworks

[Differential] [Accepted] D4444: Make tier3 dependency for tests optional

[Sune Vuorela]

svuorela accepted this revision.
svuorela added a reviewer: svuorela.
svuorela added a comment.
This revision is now accepted and ready to land.


  I'm not fully sure what the scalable test does, but having tier3 libraries 
required for tests in tier1 thingns is a bit bad to me.

REPOSITORY
  R266 Breeze Icons

BRANCH
  master

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

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

To: vonreth, sitter, svuorela
Cc: svuorela, #frameworks

[Differential] [Accepted] D4329: KMessageWidget: fix behaviour on overlapping calls of animatedShow/animatedHide

[Dominik Haumann]

dhaumann accepted this revision.
dhaumann added a comment.
This revision is now accepted and ready to land.


  KTextEditor uses KMessageWidget 4 times: floating inside on top right or 
bottom right. This is what you hacked.
  And 2 times above and below. These two times use the animation, but given the 
unit test runs fine, I think this is ok.

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  fixKMessageWidgetInstantShowHide

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

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

To: kossebau, #frameworks, dhaumann
Cc: cfeck

[Differential] [Updated] D4444: Make tier3 dependency for tests optional

[Hannah von Reth]

vonreth added a reviewer: sitter.

REPOSITORY
  R266 Breeze Icons

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

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

To: vonreth, sitter
Cc: #frameworks

[Differential] [Request, 24 lines] D4444: Make tier3 dependency for tests optional

[Hannah von Reth]

vonreth created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REPOSITORY
  R266 Breeze Icons

BRANCH
  master

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

AFFECTED FILES
  CMakeLists.txt
  autotests/CMakeLists.txt

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

To: vonreth
Cc: #frameworks

[Differential] [Closed] D4294: Latex Syntax: Fix identification of alignat environment end

[Dominik Haumann]

This revision was automatically updated to reflect the committed changes.
Closed by commit R216:540fb97a8764: LaTeX highlighting: fix alignat environment 
(authored by dhaumann).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D4294?vs=10646=10931#toc

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4294?vs=10646=10931

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

AFFECTED FILES
  autotests/folding/highlight.tex.fold
  autotests/html/highlight.tex.html
  autotests/input/highlight.tex
  autotests/reference/highlight.tex.ref
  data/syntax/latex.xml

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

To: hfujimoto, #framework_syntax_hightlighting
Cc: dhaumann, kwrite-devel, #frameworks, cullmann, vkrause

[Differential] [Commented On] D4439: KDirWatch: fix memory leak on destruction.

[David Faure]

dfaure added a comment.


  QScopedPointer wouldn't work here (this isn't about a scope). std::unique_ptr 
would most certainly work, but then again, why use pointers where values can 
work. I like Albert's approach overall, with only the two concerns I listed.

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

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

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

To: dfaure, mpyne, aacid
Cc: markg, #frameworks

Re: Review Request 129839: KFileMetaData: add a new property DiscNumber for audio files from multi-disc albums

[David Faure]

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




src/extractors/taglibextractor.cpp (line 111)


I would have initialized the int to -1, and used that as the invalid value 
(surely a disc number is never -1). It removes the risk of forgetting to set 
the bool to true in one code path.
But no big deal, not a blocker issue.



src/properties.h (line 264)


Count is supposed to be the count, and Last to be the last. Any reason why 
you're not moving DiscNumber to be before PropertyCount, as intended by this 
code?  (remove the "= ..." value)


- David Faure


On Feb. 5, 2017, 1:02 p.m., Matthieu Gallien wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129839/
> ---
> 
> (Updated Feb. 5, 2017, 1:02 p.m.)
> 
> 
> Review request for Baloo and KDE Frameworks.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> the new property is appended at the end of the existing enums such that
> binary compatibility is kept. The special values at the end of the enums
> are currently only used by automatic tests of KFileMetaData. There
> should be no harm by this commit.
> 
> At time of this commit, lxr.kde.org shows no user of the special values
> at the end of the enum. My patch should not cause any problems.
> 
> One interesting question is how to manage caching of new properties in
> Baloo when one modify KFileMetaData. I currently have no idea.
> 
> There should be more patches to add new properties after this review if 
> needed (some music related properties are still missing).
> 
> 
> Diffs
> -
> 
>   autotests/taglibextractortest.cpp d46e49ea6a189d16459799100ec49480bed893c3 
>   src/extractors/taglibextractor.cpp 8fcad93ca4fc6572a412c1f729d1ef361dd7e8cf 
>   src/properties.h 1763b9bfa4a250231932e588edbd6bebc4af3f0a 
>   src/propertyinfo.cpp 97003ae70c683eb73e2ecd84899ae35d29edaefc 
> 
> Diff: https://git.reviewboard.kde.org/r/129839/diff/
> 
> 
> Testing
> ---
> 
> Automatic tests are still all OK on my setup.
> 
> I have modified my music player to makes use of it and it works.
> 
> I have not yet any clear idea how to makes Baloo reindex the files to cache 
> new properties.
> 
> 
> Thanks,
> 
> Matthieu Gallien
> 
>

Re: Review Request 129839: KFileMetaData: add a new property DiscNumber for audio files from multi-disc albums

[Matthieu Gallien]

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

(Updated Feb. 5, 2017, 2:02 p.m.)


Review request for Baloo and KDE Frameworks.


Changes
---

Add a boolean to know if the discNumber property is valid. The musepack test 
verifies that the property is not set.


Repository: kfilemetadata


Description
---

the new property is appended at the end of the existing enums such that
binary compatibility is kept. The special values at the end of the enums
are currently only used by automatic tests of KFileMetaData. There
should be no harm by this commit.

At time of this commit, lxr.kde.org shows no user of the special values
at the end of the enum. My patch should not cause any problems.

One interesting question is how to manage caching of new properties in
Baloo when one modify KFileMetaData. I currently have no idea.

There should be more patches to add new properties after this review if needed 
(some music related properties are still missing).


Diffs (updated)
-

  autotests/taglibextractortest.cpp d46e49ea6a189d16459799100ec49480bed893c3 
  src/extractors/taglibextractor.cpp 8fcad93ca4fc6572a412c1f729d1ef361dd7e8cf 
  src/properties.h 1763b9bfa4a250231932e588edbd6bebc4af3f0a 
  src/propertyinfo.cpp 97003ae70c683eb73e2ecd84899ae35d29edaefc 

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


Testing
---

Automatic tests are still all OK on my setup.

I have modified my music player to makes use of it and it works.

I have not yet any clear idea how to makes Baloo reindex the files to cache new 
properties.


Thanks,

Matthieu Gallien

Re: Review Request 129798: add more audio formats to automatic test and fix one problem with Musepack

[Matthieu Gallien]

> On Feb. 5, 2017, 10:45 a.m., David Faure wrote:
> > Patch looks ok to me, although I don't know much about all this.
> > 
> > The real problem isn't tooling, but lack of active maintainership for 
> > kfilemetadata. Are you interested in taking over?
> 
> Matthieu Gallien wrote:
> Hello,
> 
> I have a real interest in kfilemetadata as I am using it in my audio 
> player. Still, I am not sure I have enough time for it. Not sure I would be 
> able to do better than the current situation.
> 
> What do you think ?
> 
> David Faure wrote:
> I think *any* involvement is better than the current situation :-)

OK, I can take over. I will have to find and read the list of my duties.

I believe there are a few things that would be nice like updating the 
documentation (I had to read examples to understand the APIs).


- Matthieu


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


On Jan. 15, 2017, 4:03 p.m., Matthieu Gallien wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129798/
> ---
> 
> (Updated Jan. 15, 2017, 4:03 p.m.)
> 
> 
> Review request for Baloo and KDE Frameworks.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> Musepack extraction of album artist was using the wrong tag name (tested
> against easytag and picard).
> 
> See also this page (http://taglib.org/api/classTagLib_1_1APE_1_1Tag.html) 
> that seems to indicate that APE is using "ALBUM ARTIST" with a space between 
> both words.
> 
> 
> Diffs
> -
> 
>   autotests/samplefiles/test.flac PRE-CREATION 
>   autotests/samplefiles/test.m4a PRE-CREATION 
>   autotests/samplefiles/test.mp3 PRE-CREATION 
>   autotests/samplefiles/test.mpc PRE-CREATION 
>   autotests/samplefiles/test.ogg PRE-CREATION 
>   autotests/taglibextractortest.cpp d46e49e 
>   src/extractors/taglibextractor.cpp 8fcad93 
> 
> Diff: https://git.reviewboard.kde.org/r/129798/diff/
> 
> 
> Testing
> ---
> 
> I have tested with automatic tests of KFileMetaData and also the encoding 
> problem in Musepack format against Easytag and Picard applications.
> 
> 
> Thanks,
> 
> Matthieu Gallien
> 
>

[Differential] [Commented On] D4439: KDirWatch: fix memory leak on destruction.

[Mark Gaiser]

markg added a comment.


  Hmm, this is exactly the reason why i always go for either smart pointers or 
stack objects. Both prevent this issue from occurring in the first place.
  
  It's probably a bit much to make the Entry class own the objects (basically 
the diff of Albert). But there i would be a bit worried about needless copies.
  Which you can then prohibit by not allowing copies thus forcing move 
semantics, but then QList/QVector become unusable and you'd have to switch to 
std::vector. That is probably one step too far as well.
  
  So, the last possible solution that might work is:
  QList m_clients;
  
  But i don't know enough about the Qt smart pointers to say for sure if that 
works as intended (aka, no leaks and no needless copies).

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

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

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

To: dfaure, mpyne, aacid
Cc: markg, #frameworks

[Differential] [Commented On] D4439: KDirWatch: fix memory leak on destruction.

[David Faure]

dfaure added a comment.


  It's not crazy, but
  
  - then it should use QVector instead of QList (Client is a "big" struct, 
bigger than a pointer)
  - I would be worried about copies happening unexpectedly (can this code 
compile with forbidden copy ctor for Client? I guess not as is due to insertion 
into the vector but maybe std::move can be used there, or simply setting 
the members directly onto a ref for vector[i]).

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

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

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

To: dfaure, mpyne, aacid
Cc: #frameworks

[Differential] [Accepted] D4439: KDirWatch: fix memory leak on destruction.

[Albert Astals Cid]

aacid accepted this revision.
aacid added a comment.
This revision is now accepted and ready to land.


  Looks good to me, we could also go the crazy way and hold the data in 
m_clients instead of holding the ptr to the data
  http://paste.ubuntu.com/23933091/
  
  But I'm pretty sure i did some porting mistake in there :D

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

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

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

To: dfaure, mpyne, aacid
Cc: #frameworks

Jenkins-kde-ci: kservice master stable-kf5-qt5 » Linux,gcc - Build # 235 - Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/kservice%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/235/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 05 Feb 2017 11:07:16 +
Build duration: 2 min 6 sec

CHANGE SET
No changes


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 10 test(s), Skipped: 0 test(s), Total: 
11 test(s)Failed: TestSuite.kservicetest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 6/7 (86%)FILES 75/84 (89%)CLASSES 75/84 (89%)LINE 5484/8008 
(68%)CONDITIONAL 2971/6178 (48%)

By packages
  
autotests
FILES 14/14 (100%)CLASSES 14/14 (100%)LINE 1464/1554 
(94%)CONDITIONAL 895/1792 (50%)
src.kbuildsycoca
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 61/67 (91%)CONDITIONAL 
15/20 (75%)
src.kdeinit
FILES 0/2 (0%)CLASSES 0/2 (0%)LINE 0/326 (0%)CONDITIONAL 0/262 
(0%)
src.plugin
FILES 2/3 (67%)CLASSES 2/3 (67%)LINE 47/100 (47%)CONDITIONAL 
36/96 (38%)
src.services
FILES 29/30 (97%)CLASSES 29/30 (97%)LINE 1767/3045 
(58%)CONDITIONAL 764/1892 (40%)
src.sycoca
FILES 26/31 (84%)CLASSES 26/31 (84%)LINE 2037/2796 
(73%)CONDITIONAL 1227/2066 (59%)
tests.pluginlocator
FILES 3/3 (100%)CLASSES 3/3 (100%)LINE 108/120 (90%)CONDITIONAL 
34/50 (68%)

Re: Review Request 129665: [KStatusNotifierItem] Restore mnimized window as normal

[Anthony Fieroni]

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

(Updated Feb. 5, 2017, 12:24 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Martin Klapetek.


Changes
---

Submitted with commit d2bb8062e6d21057900359333ca2bff085dcb951 by Anthony 
Fieroni to branch master.


Repository: knotifications


Description
---

I think, we want minized window to be shown as normal when it's closed in tray. 
Why we could want a window to be shown as minimized ?


Diffs
-

  src/kstatusnotifieritem.cpp fcbe6a8 

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


Testing
---

+ Remove deprecated warnings


Thanks,

Anthony Fieroni

Jenkins-kde-ci: kwindowsystem master stable-kf5-qt5 » Linux,All,gcc - Build # 149 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kwindowsystem%20master%20stable-kf5-qt5/PLATFORM=Linux,Variation=All,compiler=gcc/149/
Project: PLATFORM=Linux,Variation=All,compiler=gcc
Date of build: Sun, 05 Feb 2017 10:16:31 +
Build duration: 13 min

CHANGE SET
No changes


JUNIT RESULTS

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

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 4/5 (80%)FILES 44/50 (88%)CLASSES 44/50 (88%)LINE 6806/9081 
(75%)CONDITIONAL 3797/7069 (54%)

By packages
  
autotests
FILES 15/15 (100%)CLASSES 15/15 (100%)LINE 2940/3011 
(98%)CONDITIONAL 1616/3115 (52%)
autotests.helper
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 7/10 (70%)CONDITIONAL 
3/6 (50%)
src
FILES 14/16 (88%)CLASSES 14/16 (88%)LINE 779/1434 
(54%)CONDITIONAL 319/710 (45%)
src.platforms.wayland
FILES 0/2 (0%)CLASSES 0/2 (0%)LINE 0/89 (0%)CONDITIONAL 0/2 (0%)
src.platforms.xcb
FILES 14/16 (88%)CLASSES 14/16 (88%)LINE 3080/4537 
(68%)CONDITIONAL 1859/3236 (57%)

[Differential] [Request, 11 lines] D4439: KDirWatch: fix memory leak on destruction.

[David Faure]

dfaure created this revision.
dfaure added reviewers: aacid, mpyne.
dfaure added a subscriber: Frameworks.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  The Entry class owns the Client instances, so it should delete the
  remaining instances in its destructor, for the case where they haven't
  been removed one by one. The line of code removeEntries(nullptr) was
  probably means to remove them one by one, but it was a no-op (the code
  for that method doesn't expect nullptr as argument) and it would be
  slow anyway. We don't need to call inotify_remove for every path,
  when we're just cleaning up in a global static after qApp destruction.
  
  Detected by a clang-sanitizer build on http://ci-logs.kde.flaska.net
  and reproduced locally with valgrind.

TEST PLAN
  ./kdirwatch_*_unittest now passes in valgrind without memory
  leaks being reported

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

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

AFFECTED FILES
  src/lib/io/kdirwatch.cpp
  src/lib/io/kdirwatch_p.h

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

To: dfaure, aacid, mpyne
Cc: #frameworks

Jenkins-kde-ci: kwindowsystem master kf5-qt5 » Linux,All,gcc - Build # 151 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kwindowsystem%20master%20kf5-qt5/PLATFORM=Linux,Variation=All,compiler=gcc/151/
Project: PLATFORM=Linux,Variation=All,compiler=gcc
Date of build: Sun, 05 Feb 2017 10:16:30 +
Build duration: 1 min 46 sec

CHANGE SET
No changes


JUNIT RESULTS

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

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 4/5 (80%)FILES 44/50 (88%)CLASSES 44/50 (88%)LINE 6806/9081 
(75%)CONDITIONAL 3795/7069 (54%)

By packages
  
autotests
FILES 15/15 (100%)CLASSES 15/15 (100%)LINE 2940/3011 
(98%)CONDITIONAL 1615/3115 (52%)
autotests.helper
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 7/10 (70%)CONDITIONAL 
3/6 (50%)
src
FILES 14/16 (88%)CLASSES 14/16 (88%)LINE 778/1434 
(54%)CONDITIONAL 317/710 (45%)
src.platforms.wayland
FILES 0/2 (0%)CLASSES 0/2 (0%)LINE 0/89 (0%)CONDITIONAL 0/2 (0%)
src.platforms.xcb
FILES 14/16 (88%)CLASSES 14/16 (88%)LINE 3081/4537 
(68%)CONDITIONAL 1860/3236 (57%)

Jenkins-kde-ci: kwindowsystem master kf5-qt5 » Linux,All,gcc - Build # 151 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kwindowsystem%20master%20kf5-qt5/PLATFORM=Linux,Variation=All,compiler=gcc/151/
Project: PLATFORM=Linux,Variation=All,compiler=gcc
Date of build: Sun, 05 Feb 2017 10:16:30 +
Build duration: 1 min 46 sec

CHANGE SET
No changes


JUNIT RESULTS

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

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 4/5 (80%)FILES 44/50 (88%)CLASSES 44/50 (88%)LINE 6806/9081 
(75%)CONDITIONAL 3795/7069 (54%)

By packages
  
autotests
FILES 15/15 (100%)CLASSES 15/15 (100%)LINE 2940/3011 
(98%)CONDITIONAL 1615/3115 (52%)
autotests.helper
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 7/10 (70%)CONDITIONAL 
3/6 (50%)
src
FILES 14/16 (88%)CLASSES 14/16 (88%)LINE 778/1434 
(54%)CONDITIONAL 317/710 (45%)
src.platforms.wayland
FILES 0/2 (0%)CLASSES 0/2 (0%)LINE 0/89 (0%)CONDITIONAL 0/2 (0%)
src.platforms.xcb
FILES 14/16 (88%)CLASSES 14/16 (88%)LINE 3081/4537 
(68%)CONDITIONAL 1860/3236 (57%)

Jenkins-kde-ci: kitemviews master kf5-qt5 » Linux,gcc - Build # 130 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kitemviews%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/130/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 05 Feb 2017 10:16:02 +
Build duration: 10 min

CHANGE SET
No changes


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 2/17 (12%)CLASSES 2/17 (12%)LINE 101/1931 
(5%)CONDITIONAL 40/1130 (4%)

By packages
  
autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 23/23 (100%)CONDITIONAL 
15/28 (54%)
src
FILES 1/16 (6%)CLASSES 1/16 (6%)LINE 78/1908 (4%)CONDITIONAL 
25/1102 (2%)

Jenkins-kde-ci: kitemviews master kf5-qt5 » Linux,gcc - Build # 130 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kitemviews%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/130/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 05 Feb 2017 10:16:02 +
Build duration: 10 min

CHANGE SET
No changes


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 2/17 (12%)CLASSES 2/17 (12%)LINE 101/1931 
(5%)CONDITIONAL 40/1130 (4%)

By packages
  
autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 23/23 (100%)CONDITIONAL 
15/28 (54%)
src
FILES 1/16 (6%)CLASSES 1/16 (6%)LINE 78/1908 (4%)CONDITIONAL 
25/1102 (2%)

Re: Review Request 129798: add more audio formats to automatic test and fix one problem with Musepack

[David Faure]

> On Feb. 5, 2017, 9:45 a.m., David Faure wrote:
> > Patch looks ok to me, although I don't know much about all this.
> > 
> > The real problem isn't tooling, but lack of active maintainership for 
> > kfilemetadata. Are you interested in taking over?
> 
> Matthieu Gallien wrote:
> Hello,
> 
> I have a real interest in kfilemetadata as I am using it in my audio 
> player. Still, I am not sure I have enough time for it. Not sure I would be 
> able to do better than the current situation.
> 
> What do you think ?

I think *any* involvement is better than the current situation :-)


- David


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


On Jan. 15, 2017, 3:03 p.m., Matthieu Gallien wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129798/
> ---
> 
> (Updated Jan. 15, 2017, 3:03 p.m.)
> 
> 
> Review request for Baloo and KDE Frameworks.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> Musepack extraction of album artist was using the wrong tag name (tested
> against easytag and picard).
> 
> See also this page (http://taglib.org/api/classTagLib_1_1APE_1_1Tag.html) 
> that seems to indicate that APE is using "ALBUM ARTIST" with a space between 
> both words.
> 
> 
> Diffs
> -
> 
>   autotests/samplefiles/test.flac PRE-CREATION 
>   autotests/samplefiles/test.m4a PRE-CREATION 
>   autotests/samplefiles/test.mp3 PRE-CREATION 
>   autotests/samplefiles/test.mpc PRE-CREATION 
>   autotests/samplefiles/test.ogg PRE-CREATION 
>   autotests/taglibextractortest.cpp d46e49e 
>   src/extractors/taglibextractor.cpp 8fcad93 
> 
> Diff: https://git.reviewboard.kde.org/r/129798/diff/
> 
> 
> Testing
> ---
> 
> I have tested with automatic tests of KFileMetaData and also the encoding 
> problem in Musepack format against Easytag and Picard applications.
> 
> 
> Thanks,
> 
> Matthieu Gallien
> 
>

Re: Review Request 129798: add more audio formats to automatic test and fix one problem with Musepack

[Matthieu Gallien]

> On Feb. 5, 2017, 10:45 a.m., David Faure wrote:
> > Patch looks ok to me, although I don't know much about all this.
> > 
> > The real problem isn't tooling, but lack of active maintainership for 
> > kfilemetadata. Are you interested in taking over?

Hello,

I have a real interest in kfilemetadata as I am using it in my audio player. 
Still, I am not sure I have enough time for it. Not sure I would be able to do 
better than the current situation.

What do you think ?


- Matthieu


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


On Jan. 15, 2017, 4:03 p.m., Matthieu Gallien wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129798/
> ---
> 
> (Updated Jan. 15, 2017, 4:03 p.m.)
> 
> 
> Review request for Baloo and KDE Frameworks.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> Musepack extraction of album artist was using the wrong tag name (tested
> against easytag and picard).
> 
> See also this page (http://taglib.org/api/classTagLib_1_1APE_1_1Tag.html) 
> that seems to indicate that APE is using "ALBUM ARTIST" with a space between 
> both words.
> 
> 
> Diffs
> -
> 
>   autotests/samplefiles/test.flac PRE-CREATION 
>   autotests/samplefiles/test.m4a PRE-CREATION 
>   autotests/samplefiles/test.mp3 PRE-CREATION 
>   autotests/samplefiles/test.mpc PRE-CREATION 
>   autotests/samplefiles/test.ogg PRE-CREATION 
>   autotests/taglibextractortest.cpp d46e49e 
>   src/extractors/taglibextractor.cpp 8fcad93 
> 
> Diff: https://git.reviewboard.kde.org/r/129798/diff/
> 
> 
> Testing
> ---
> 
> I have tested with automatic tests of KFileMetaData and also the encoding 
> problem in Musepack format against Easytag and Picard applications.
> 
> 
> Thanks,
> 
> Matthieu Gallien
> 
>

Re: Review Request 129921: [kio-extras] thumbnails should be a clean image representation without "hard-coded" borders or design elements

[David Faure]

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


Ship it!




Can you also deprecate DrawFrame, then?

- David Faure


On Feb. 4, 2017, 9:51 p.m., Diego S. wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129921/
> ---
> 
> (Updated Feb. 4, 2017, 9:51 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio-extras
> 
> 
> Description
> ---
> 
> This patch is part of https://git.reviewboard.kde.org/r/129918/ - see the 
> discussion on the old patch https://git.reviewboard.kde.org/r/118961/.
> 
> Many thumbnail generators currently return ThumbCreator::DrawFrame in their 
> flags() function which causes a 90's style 1px "3d" border to be added to the 
> thumbnail image.
> This causes problems when consumers of these thumbnails (like Dolphin) expect 
> a clean image representation of the file, without hard-coded design elements.
> It also looks really bad when any resizing/scaling happens or when the 
> thumbnail is shown on a tooltip. Especially now that the UI has been 
> modernized with Breeze.
> 
> 
> Diffs
> -
> 
>   thumbnail/comiccreator.cpp d8f4473e1c7fa4595ad8defc406ad945d1ac38ac 
>   thumbnail/djvucreator.cpp 1547831bfaa875a14a59659a2bebc86a2cd7c024 
>   thumbnail/htmlcreator.cpp 29d1902794abfa3cf8ba2ab97648e1bc2d8e7a73 
>   thumbnail/kritacreator.cpp 3b24bc3d2fa7b22662c25da1421cbd542d7efed6 
>   thumbnail/textcreator.cpp 7c0263c8e8cb4f88d1429b5714c51c2939d817f0 
> 
> Diff: https://git.reviewboard.kde.org/r/129921/diff/
> 
> 
> Testing
> ---
> 
> Builds.
> 
> 
> Thanks,
> 
> Diego S.
> 
>

Re: Review Request 129911: Added property() method to TerminalInterface class

[David Faure]

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



Alas, this is a binary incompatible change. It cannot be done this way.
Solution 1: a V2 interface inheriting from this one (as we had in the past, 
IIRC).
Solution 2: just use QObject dynamic properties, after documenting that in this 
header file.

- David Faure


On Feb. 3, 2017, 5:23 p.m., Sven Fischer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129911/
> ---
> 
> (Updated Feb. 3, 2017, 5:23 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kparts
> 
> 
> Description
> ---
> 
> For konsole KPart consumers it may be necessary to access the properties
> of the konsole profile, e.g. the "Start in current session dir" to be
> able to control the working directory in new KPart instantiation.
> 
> A corresponding patch has been submitted to the konsole repository.
> 
> 
> Diffs
> -
> 
>   src/kde_terminal_interface.h f9603d120d5116db35ae60d65b2743a5aceaebac 
> 
> Diff: https://git.reviewboard.kde.org/r/129911/diff/
> 
> 
> Testing
> ---
> 
> Locally compile konsole against this changed header, and running a yakuake 
> instance against the patched konsole.
> 
> 
> Thanks,
> 
> Sven Fischer
> 
>

Re: Review Request 129665: [KStatusNotifierItem] Restore mnimized window as normal

[David Faure]

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


Ship it!




Ship It!

- David Faure


On Feb. 2, 2017, 11:13 a.m., Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129665/
> ---
> 
> (Updated Feb. 2, 2017, 11:13 a.m.)
> 
> 
> Review request for KDE Frameworks and Martin Klapetek.
> 
> 
> Repository: knotifications
> 
> 
> Description
> ---
> 
> I think, we want minized window to be shown as normal when it's closed in 
> tray. Why we could want a window to be shown as minimized ?
> 
> 
> Diffs
> -
> 
>   src/kstatusnotifieritem.cpp fcbe6a8 
> 
> Diff: https://git.reviewboard.kde.org/r/129665/diff/
> 
> 
> Testing
> ---
> 
> + Remove deprecated warnings
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

Re: Review Request 129798: add more audio formats to automatic test and fix one problem with Musepack

[David Faure]

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


Ship it!




Patch looks ok to me, although I don't know much about all this.

The real problem isn't tooling, but lack of active maintainership for 
kfilemetadata. Are you interested in taking over?

- David Faure


On Jan. 15, 2017, 3:03 p.m., Matthieu Gallien wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129798/
> ---
> 
> (Updated Jan. 15, 2017, 3:03 p.m.)
> 
> 
> Review request for Baloo and KDE Frameworks.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> Musepack extraction of album artist was using the wrong tag name (tested
> against easytag and picard).
> 
> See also this page (http://taglib.org/api/classTagLib_1_1APE_1_1Tag.html) 
> that seems to indicate that APE is using "ALBUM ARTIST" with a space between 
> both words.
> 
> 
> Diffs
> -
> 
>   autotests/samplefiles/test.flac PRE-CREATION 
>   autotests/samplefiles/test.m4a PRE-CREATION 
>   autotests/samplefiles/test.mp3 PRE-CREATION 
>   autotests/samplefiles/test.mpc PRE-CREATION 
>   autotests/samplefiles/test.ogg PRE-CREATION 
>   autotests/taglibextractortest.cpp d46e49e 
>   src/extractors/taglibextractor.cpp 8fcad93 
> 
> Diff: https://git.reviewboard.kde.org/r/129798/diff/
> 
> 
> Testing
> ---
> 
> I have tested with automatic tests of KFileMetaData and also the encoding 
> problem in Musepack format against Easytag and Picard applications.
> 
> 
> Thanks,
> 
> Matthieu Gallien
> 
>

[Differential] [Updated] D4431: Add KUrlRequester::setMimeTypeFilters.

[David Faure]

dfaure marked an inline comment as done.
dfaure added inline comments.

INLINE COMMENTS

> cfeck wrote in kurlrequester.h:144
> The APIDOX for these two functions are somehow swapped.

Thanks for noticing, fixed.

REPOSITORY
  R241 KIO

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

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

To: dfaure, mlaurent, markg
Cc: cfeck, markg, #frameworks

[Differential] [Closed] D4247: KIconEngine: Center icon in requested rect

[David Rosca]

This revision was automatically updated to reflect the committed changes.
Closed by commit R302:456b57d71c67: KIconEngine: Center icon in requested rect 
(authored by drosca).

REPOSITORY
  R302 KIconThemes

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4247?vs=10438=10923

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

AFFECTED FILES
  autotests/kiconengine_unittest.cpp
  src/kiconengine.cpp

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

To: drosca, #frameworks, dfaure