Re: Review Request 128397: KIconEngine: Fix QIcon::hasThemeIcon always returning true

2016-07-12 Thread David Rosca
marked as submitted. Review request for KDE Frameworks and Olivier Goffart. Changes --- Submitted with commit 0abf1b7a148cf6b27caea01a329631e0f1daa983 by David Rosca to branch master. Bugs: 365130 https://bugs.kde.org/show_bug.cgi?id=365130 Repository: kiconthemes Description

Re: Review Request 128580: If we pass a QIcon as an argument to IconItem::Source, use it

2016-08-03 Thread David Rosca
a.png file representing completely different icon. In this case, however, it would fail in the codepath above - being loaded either from plasma theme or svg with kiconloader. - David Rosca On Aug. 2, 2016, 9:11 p.m., David Edmun

Re: Review Request 128580: If we pass a QIcon as an argument to IconItem::Source, use it

2016-08-03 Thread David Rosca
> On Aug. 3, 2016, 7:38 a.m., David Rosca wrote: > > Why does it break for SNIs? It first tries QIcon::fromTheme and if it fails > > (isNull()) fallbacks to original QIcon. This patch just switched it, why > > does it make a difference? > > > > The problem I

Re: Review Request 128580: If we pass a QIcon as an argument to IconItem::Source, use it

2016-08-03 Thread David Rosca
> On Aug. 3, 2016, 7:38 a.m., David Rosca wrote: > > Why does it break for SNIs? It first tries QIcon::fromTheme and if it fails > > (isNull()) fallbacks to original QIcon. This patch just switched it, why > > does it make a difference? > > > > The problem I

Re: Review Request 128397: KIconEngine: Fix QIcon::hasThemeIcon always returning true

2016-07-08 Thread David Rosca
/browse/QTBUG-54595 Also similar issue https://bugs.kde.org/show_bug.cgi?id=365031 (there is a check for hasThemeIcon before using the icon, but it returns true which results in invalid icon name -> no icon). Thanks, David Rosca ___ Kde-frameworks-de

Re: Review Request 128397: KIconEngine: Fix QIcon::hasThemeIcon always returning true

2016-07-08 Thread David Rosca
tps://bugreports.qt.io/browse/QTBUG-54595 Also similar issue https://bugs.kde.org/show_bug.cgi?id=365031 (there is a check for hasThemeIcon before using the icon, but it returns true which results in invalid icon name -> no icon). Thanks, David

Review Request 128397: KIconEngine: Fix QIcon::hasThemeIcon always returning true

2016-07-07 Thread David Rosca
using the icon, but it returns true which results in invalid icon name -> no icon). Thanks, David Rosca ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128397: KIconEngine: Fix QIcon::hasThemeIcon always returning true

2016-07-08 Thread David Rosca
mail. To reply, visit: https://git.reviewboard.kde.org/r/128397/#review97181 ------- On July 7, 2016, 9:55 p.m., David Rosca wrote: > > --- > This is an automa

Re: Review Request 128340: Don't use QQuickWidget::quickWindow() as it was added in Qt 5.5

2016-07-02 Thread David Rosca
ard.kde.org/r/128340/#review97009 --- On July 2, 2016, 8:44 a.m., David Rosca wrote: > > --- > This is an automatically generated e-mail. To reply, visit:

Re: Review Request 128340: Don't use QQuickWidget::quickWindow() as it was added in Qt 5.5

2016-07-02 Thread David Rosca
--- Should build fine with Qt 5.4 according to docs. Not tested myself though. Thanks, David Rosca ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127469: Use QQuickWidget for QML KCMs

2016-07-02 Thread David Rosca
omatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127469/#review97002 ------- On May 20, 2016, 1:33 p.m., David Rosca wrote: > > --- > This is

Review Request 128340: Don't use QQuickWidget::quickWindow() as it was added in Qt 5.5

2016-07-02 Thread David Rosca
, David Rosca ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128340: Don't use QQuickWidget::quickWindow() as it was added in Qt 5.5

2016-07-03 Thread David Rosca
marked as submitted. Review request for KDE Frameworks and David Faure. Changes --- Submitted with commit 5432c3edf5e074f1e951e6ecc682f7a400e2818f by David Rosca to branch master. Repository: kcmutils Description --- Fixes build with Qt 5.4 after

[Differential] [Commented On] D4247: KIconEngine: Center icon in requested rect

2017-02-02 Thread David Rosca
drosca added a comment. Ping REPOSITORY R302 KIconThemes REVISION DETAIL https://phabricator.kde.org/D4247 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: drosca, #frameworks

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

2017-02-05 Thread 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

[Differential] [Request, 10 lines] D4247: KIconEngine: Center icon in requested rect

2017-01-22 Thread David Rosca
drosca created this revision. drosca added a reviewer: Frameworks. Restricted Application added a project: Frameworks. REVISION SUMMARY Match the behavior of Qt's internal icon engines. TEST PLAN Rendering is the same for unaffected icons + highdpi still works. Example case: Render icon

[Differential] [Updated, 25 lines] D4247: KIconEngine: Center icon in requested rect

2017-01-22 Thread David Rosca
drosca updated this revision to Diff 10438. drosca added a comment. Add autotest REPOSITORY R302 KIconThemes CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D4247?vs=10437=10438 BRANCH master REVISION DETAIL https://phabricator.kde.org/D4247 AFFECTED FILES

[Differential] [Closed] D4282: Dialog: Hide when focus changes to ConfigView with hideOnWindowDeactivate

2017-01-26 Thread David Rosca
This revision was automatically updated to reflect the committed changes. Closed by commit R242:d2c7435b1bc1: Dialog: Hide when focus changes to ConfigView with hideOnWindowDeactivate (authored by drosca). REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE

[Differential] [Request, 3 lines] D4282: Dialog: Hide when focus changes to ConfigView with hideOnWindowDeactivate

2017-01-25 Thread David Rosca
drosca created this revision. drosca added a reviewer: Plasma. Restricted Application added projects: Plasma, Frameworks. Restricted Application added subscribers: Frameworks, plasma-devel. TEST PLAN Expand applet in systray -> open config -> applet popup gets closed REPOSITORY R242 Plasma

[Differential] [Commented On] D4689: IconItem: Add roundToIconSize property

2017-02-21 Thread David Rosca
drosca added a comment. Yes, I want to use it in plasma-pa applet for volume indicator icons (the small icon next to slider). Currently, they are too small but next round icon size is already too big. This change will make it possible to make them just few pixels bigger. REPOSITORY R242

[Differential] [Closed] D4702: IconItemTest: Fix loadPixmap and loadImage tests

2017-02-21 Thread David Rosca
This revision was automatically updated to reflect the committed changes. Closed by commit R242:148ff6311bfe: IconItemTest: Fix loadPixmap and loadImage tests (authored by drosca). REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE

[Differential] [Request, 2 lines] D4702: IconItemTest: Fix loadPixmap and loadImage tests

2017-02-21 Thread David Rosca
drosca created this revision. Restricted Application added projects: Plasma, Frameworks. Restricted Application added subscribers: Frameworks, plasma-devel. REVISION SUMMARY It only works on CI because data/test_image.png size is the same as implicitSize of IconItem (KIconLoader::Dialog).

[Differential] [Updated] D4688: [FrameSvgItemMargins] Don't update on repaintNeeded

2017-02-20 Thread David Rosca
drosca added a comment. Looks fine, but unfortunately it doesn't help with anything, the issue with networkmanager applet delegates is still there. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D4688 EMAIL PREFERENCES

[Differential] [Request, 56 lines] D4689: IconItem: Add roundToIconSize property

2017-02-20 Thread David Rosca
drosca created this revision. Restricted Application added projects: Plasma, Frameworks. Restricted Application added subscribers: Frameworks, plasma-devel. REVISION SUMMARY Disabling this property makes it possible to show icon of arbitrary size. TEST PLAN Test passed REPOSITORY R242

[Differential] [Commented On] D4689: IconItem: Add roundToIconSize property

2017-02-22 Thread David Rosca
drosca added a comment. > they may end up looking a bit blurry They are SVGs in Breeze, so they won't look blurry, with other icon themes it may be an issue. But then again, the same issue is already there when you request big icon (for which roundToIconSize() returns the passed size ~

[Differential] [Commented On] D4342: Use texture atlas for static icon item

2017-02-14 Thread David Rosca
drosca added a comment. This broke rendering of icons in Quicklaunch plasmoid for me (Sandy Bridge GPU), you can see on the screenshot that it is pixelated. Also the image slightly moves during the active animation, as texture atlas is used only when animation is not running. I can only

[Differential] [Updated] D4342: Use texture atlas for static icon item

2017-02-14 Thread David Rosca
drosca added a comment. Yes. Latest master https://phabricator.kde.org/R242:263f119e17df7c24f8372710c555486429b57971 - broken. Latest master https://phabricator.kde.org/R242:263f119e17df7c24f8372710c555486429b57971 with this change reverted - fixed. REPOSITORY R242 Plasma

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

2017-02-15 Thread David Rosca
drosca added a comment. I now get tons of binding loop errors from FrameSvgItem, it also breaks delegates in networkmanager and bluetooth applets. On the screenshot you can see that the last 3 delegates (HUAWEI, UPC and Internet) are slightly moved to the left and hovering over them will

[Differential] [Closed] D4646: Manager: Fix emitting deviceAdded twice when NM restarts

2017-02-17 Thread David Rosca
This revision was automatically updated to reflect the committed changes. Closed by commit R282:a183e1f346a4: Manager: Fix emitting deviceAdded twice when NM restarts (authored by drosca). REPOSITORY R282 NetworkManagerQt CHANGES SINCE LAST UPDATE

[Differential] [Request, 62 lines] D4646: Manager: Fix emitting deviceAdded twice when NM restarts

2017-02-17 Thread David Rosca
drosca created this revision. drosca added reviewers: Frameworks, jgrulich. Restricted Application added a project: Frameworks. TEST PLAN Restart NetworkManager -> deviceAdded only once for each device REPOSITORY R282 NetworkManagerQt BRANCH master REVISION DETAIL

[Differential] [Commented On] D4689: IconItem: Add roundToIconSize property

2017-02-27 Thread David Rosca
drosca added inline comments. INLINE COMMENTS > sebas wrote in iconitem.cpp:313 > Changing roundToIconSize may change the size of the icon, so should we > trigger size changes (paintedWidth / paintedHeight / boundingRect likely? > Perhaps others.) and re-rendering here as well? Right now,

[Differential] [Updated, 68 lines] D4689: IconItem: Add roundToIconSize property

2017-02-27 Thread David Rosca
drosca updated this revision to Diff 11897. drosca added a comment. Rename getter Emit paintedSizeChanged + schedulePixmapUpdate REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D4689?vs=11845=11897 BRANCH arcpatch-D4689 REVISION

[Differential] [Closed] D4689: IconItem: Add roundToIconSize property

2017-02-28 Thread David Rosca
This revision was automatically updated to reflect the committed changes. Closed by commit R242:af2b27d1b89b: IconItem: Add roundToIconSize property (authored by drosca). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D4689?vs=11897=11934#toc REPOSITORY R242 Plasma Framework (Library)

[Differential] [Updated, 56 lines] D4689: IconItem: Add roundToIconSize property

2017-02-26 Thread David Rosca
drosca updated this revision to Diff 11845. drosca added a comment. Add default is true REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D4689?vs=11551=11845 BRANCH arcpatch-D4689 REVISION DETAIL https://phabricator.kde.org/D4689

Re: Review Request 128981: Drop obsolete version check

2016-09-21 Thread David Rosca
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128981/#review99382 --- Ship it! Ship It! - David Rosca On Sept. 21, 2016, 4

Re: Review Request 129302: Fix include dir in pri file

2016-11-01 Thread David Rosca
marked as submitted. Review request for KDE Frameworks and Jan Grulich. Changes --- Submitted with commit 4e295bcf03d3964d5675894053190a71a5288baa by David Rosca to branch master. Repository: networkmanager-qt Description --- Currently the pri file has Qt.NetworkManagerQt.includes

Review Request 129303: Fix include dir in pri file

2016-11-01 Thread David Rosca
--- Correct include paths when used from qmake. Thanks, David Rosca

Review Request 129302: Fix include dir in pri file

2016-11-01 Thread David Rosca
--- Correct include paths when used from qmake. Thanks, David Rosca

Re: Review Request 129302: Fix include dir in pri file

2016-11-01 Thread David Rosca
://git.reviewboard.kde.org/r/129302/diff/ Testing (updated) --- Correct include paths when used from qmake. It still cannot be used from qmake without additional changes because `generictypes.h` includes `nm-version.h` from libnm which is not in include paths. Thanks, David Rosca

Re: Review Request 129303: Fix include dir in pri file

2016-11-01 Thread David Rosca
marked as submitted. Review request for KDE Frameworks and Jan Grulich. Changes --- Submitted with commit 79725bf9892520538a6bde836ced583a65ea6767 by David Rosca to branch master. Repository: modemmanager-qt Description --- Currently the pri file has Qt.ModemManagerQt.includes

Re: Review Request 129086: Update applet alternatives menu entry visibility on demand

2016-10-26 Thread David Rosca
> On Oct. 26, 2016, 10:36 a.m., David Rosca wrote: > > This made the Alternatives action visible in toolbox menu. > > > > The problem is that toolbox tries to emit `contextualActionsAboutToShow` on > > `AppletInterface`, but the connection between > > `App

Re: Review Request 129086: Update applet alternatives menu entry visibility on demand

2016-10-26 Thread David Rosca
`, not the other way). This never worked (emitting `contextualActionsAboutToShow` from plasmoid), but only this change exposed it. I have no idea how to fix it, other than adding new method to `AppletInterface` to emit this signal or hacky both-way connection. - David Rosca On Oct. 3, 2016, 10:45 a.m

Re: Review Request 129743: Q_ENUMS -> Q_ENUM

2017-01-02 Thread David Rosca
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129743/#review101719 --- Ship it! Ship It! - David Rosca On Jan. 2, 2017, 10

D5066: Provide device type for Low Energy devices

2017-03-16 Thread David Rosca
drosca accepted this revision. This revision is now accepted and ready to land. REPOSITORY R269 BluezQt BRANCH master REVISION DETAIL https://phabricator.kde.org/D5066 To: elvisangelaccio, drosca Cc: #frameworks

D5324: Revert "[calendar] Use ui language for getting the month name"

2017-04-06 Thread David Rosca
drosca abandoned this revision. drosca added a comment. > However, if your UI language is all Spanish and you want to use the US date formatting, the month name labels would suddenly be in English while they should remain Spanish (and this is what the code you're removing does). Well,

D5324: Revert "[calendar] Use ui language for getting the month name"

2017-04-06 Thread David Rosca
drosca created this revision. Restricted Application added projects: Plasma, Frameworks. Restricted Application added subscribers: Frameworks, plasma-devel. REVISION SUMMARY Qt.locale().standaloneMonthName() is used from QML side in MonthView, so using the same code on C++ part again makes

D5345: Calendar: Use correct language for month and day names

2017-04-11 Thread David Rosca
This revision was automatically updated to reflect the committed changes. Closed by commit R242:a01e4fb69efe: Calendar: Use correct language for month and day names (authored by drosca). REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE

D5345: Calendar: Use correct language for month and day names

2017-04-11 Thread David Rosca
drosca added inline comments. INLINE COMMENTS > mck182 wrote in DaysCalendar.qml:315 > Now I'm not entirely sure but can the `uiLanguages[0]` > possibly return null? It's used exactly the same on the C++ side (without the bounds check), so I assume it's safe as that code is now over 2 years

D5550: Fix property changes being missed immediately after an obejct is added

2017-04-23 Thread David Rosca
drosca created this revision. Restricted Application added a project: Frameworks. REVISION SUMMARY Fix race condition when property changes may be missed if the property is changed immediately after the object is created. The issue was that the connection to PropertyChanged signal was

D5345: Calendar: Use correct language for month and day names

2017-04-08 Thread David Rosca
drosca created this revision. Restricted Application added projects: Plasma, Frameworks. Restricted Application added subscribers: Frameworks, plasma-devel. REVISION SUMMARY Apply fix for bug 353715 also on QML side. TEST PLAN I have English ui language + Czech time format. Months and days

Re: KDE CI: Frameworks bluez-qt kf5-qt5 XenialQt5.7 - Build # 20 - Unstable!

2017-08-14 Thread David Rosca
Hi, I just fixed it yesterday and now it fails again. What did change this time, please? Thanks, David On Mon, Aug 14, 2017 at 3:54 PM, wrote: > *BUILD UNSTABLE* > Build URL https://build.kde.org/job/Frameworks%20bluez-qt%20kf5- > qt5%20XenialQt5.7/20/ > Project: Frameworks

D5550: Fix property changes being missed immediately after an obejct is added

2017-05-26 Thread David Rosca
This revision was automatically updated to reflect the committed changes. Closed by commit R269:c02c4806c9bf: Fix property changes being missed immediately after an obejct is added (authored by drosca). REPOSITORY R269 BluezQt CHANGES SINCE LAST UPDATE

D8806: Do not leak rfkill file descriptors.

2017-11-14 Thread David Rosca
drosca added a comment. Alright, thanks for the explanation. I need your full name and e-mail to commit it (I can't get it with `arch patch` probably because you didn't upload this review with `arc diff`). REPOSITORY R269 BluezQt REVISION DETAIL https://phabricator.kde.org/D8806

D8806: Do not leak rfkill file descriptors.

2017-11-14 Thread David Rosca
This revision was automatically updated to reflect the committed changes. Closed by commit R269:ae044d6dfec9: Do not leak rfkill file descriptors. (authored by ofreyermuth, committed by drosca). REPOSITORY R269 BluezQt CHANGES SINCE LAST UPDATE

D8806: Do not leak rfkill file descriptors.

2017-11-14 Thread David Rosca
drosca added a comment. In https://phabricator.kde.org/D8806#167509, @bshah wrote: > I do wonder if this is security fix? And should probably be handled like that? Why? It's not like any other process can't open /dev/rfkill as we ship udev rule to enable r+w access to

D8806: Do not leak rfkill file descriptors.

2017-11-13 Thread David Rosca
drosca added a comment. Just for the record, how does Konsole inherit this fd when BluezQt is only used in plasmashell + kded and KDE apps are afaik not forked from these processes? REPOSITORY R269 BluezQt REVISION DETAIL https://phabricator.kde.org/D8806 To: ofreyermuth,

D8806: Do not leak rfkill file descriptors.

2017-11-14 Thread David Rosca
drosca added a comment. Though on the other hand, why isn't KLauncher (KRun) used for running apps everywhere? This way they are forked from `kdeinit5` and this issue is non-existant. Quick test: - Launch from taskmanager -> forked from plasmashell - Launch from quicklaunch

D13268: Add signal for devices's address changing

2018-06-04 Thread David Rosca
drosca added a comment. Do you have a dev account? REPOSITORY R269 BluezQt REVISION DETAIL https://phabricator.kde.org/D13268 To: andreysemenov, drosca Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13268: Add signal for devices's address changing

2018-06-04 Thread David Rosca
drosca added a comment. In D13268#273516 , @andreysemenov wrote: > In D13268#273515 , @drosca wrote: > > > Do you have a dev account? > > > I think i don't. Ok, your name and e-mail

D13268: Add signal for devices's address changing

2018-06-04 Thread David Rosca
This revision was automatically updated to reflect the committed changes. Closed by commit R269:41b41939c028: Add signal for devicess address changing (authored by andreysemenov, committed by drosca). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D13268?vs=35331=35511#toc REPOSITORY

D13268: Add signal for devices's address changing

2018-06-01 Thread David Rosca
drosca accepted this revision. This revision is now accepted and ready to land. REPOSITORY R269 BluezQt REVISION DETAIL https://phabricator.kde.org/D13268 To: andreysemenov, mlaurent, drosca Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D10167: Initialize m_usableAdapter with nullptr

2018-01-28 Thread David Rosca
drosca accepted this revision. This revision is now accepted and ready to land. REPOSITORY R269 BluezQt BRANCH master REVISION DETAIL https://phabricator.kde.org/D10167 To: aacid, dhaumann, drosca Cc: dhaumann, drosca, #frameworks, michaelh

D10167: Initialize m_usableAdapter with nullptr

2018-01-28 Thread David Rosca
drosca requested changes to this revision. drosca added a comment. This revision now requires changes to proceed. Since it is a shared pointer then it doesn't need explicit initialization and this line can just be removed. REPOSITORY R269 BluezQt REVISION DETAIL

D16084: Add Media and MediaEndpoint API header generation

2018-10-23 Thread David Rosca
This revision was automatically updated to reflect the committed changes. Closed by commit R269:a65823a9aa43: Add Media and MediaEndpoint API header generation (authored by mweichselbaumer, committed by drosca). REPOSITORY R269 BluezQt CHANGES SINCE LAST UPDATE

D16084: Add Media and MediaEndpoint API header generation

2018-10-23 Thread David Rosca
drosca accepted this revision. This revision is now accepted and ready to land. REPOSITORY R269 BluezQt REVISION DETAIL https://phabricator.kde.org/D16084 To: mweichselbaumer, drosca Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15745: Implement Media and MediaEndpoint API

2018-10-04 Thread David Rosca
drosca accepted this revision. drosca added a comment. This revision is now accepted and ready to land. Remove NoInputNoOutputAgent and it's good to go. REVISION DETAIL https://phabricator.kde.org/D15745 To: mweichselbaumer, drosca Cc: broulik, kde-frameworks-devel, michaelh, ngraham,

D15745: Implement Media and MediaEndpoint API

2018-10-04 Thread David Rosca
drosca added a comment. Alright, last thing: Why NoInputNoOutputAgent? That should be implemented by the application, and not be part of library. In almost all cases you actually want to inform user that something is trying to connect anyway. REVISION DETAIL

D15745: Implement Media and MediaEndpoint API

2018-10-04 Thread David Rosca
drosca added a comment. In D15745#336662 , @mweichselbaumer wrote: > In D15745#336644 , @drosca wrote: > > > Remove NoInputNoOutputAgent and it's good to go. > > > Agree. Is it ok to move it

D15745: Implement Media and MediaEndpoint API

2018-10-04 Thread David Rosca
drosca added a comment. In D15745#336615 , @mweichselbaumer wrote: > In D15745#336593 , @drosca wrote: > > > Alright, last thing: > > > > Why NoInputNoOutputAgent? That should be implemented by

D15745: Implement Media and MediaEndpoint API

2018-10-04 Thread David Rosca
drosca added a comment. Thanks. I'll need your full name + e-mail if you don't have dev account to push it. REVISION DETAIL https://phabricator.kde.org/D15745 To: mweichselbaumer, drosca Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns

D15745: Implement Media and MediaEndpoint API

2018-10-04 Thread David Rosca
This revision was automatically updated to reflect the committed changes. Closed by commit R269:5f12404807cc: Implement Media and MediaEndpoint API (authored by mweichselbaumer, committed by drosca). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D15745?vs=42877=42878#toc REPOSITORY

D15745: Implement Media and MediaEndpoint API

2018-10-04 Thread David Rosca
drosca requested changes to this revision. drosca added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > manager_p.cpp:46 > , m_bluezProfileManager(nullptr) > +, m_media(nullptr) > , m_initialized(false) No need because it is smart pointer. >

D15745: Implement Media and MediaEndpoint API

2018-09-27 Thread David Rosca
drosca added inline comments. INLINE COMMENTS > mweichselbaumer wrote in media_p.h:37 > MediaPrivate will later act as parent for child objects (inheriting QObject). Then just parent them to Media instead of private class. REVISION DETAIL https://phabricator.kde.org/D15745 To:

D15745: Implement Media and MediaEndpoint API

2018-09-25 Thread David Rosca
drosca requested changes to this revision. drosca added a comment. This revision now requires changes to proceed. Looks good apart from the coding style. Also it would be great to have at least basic autotest. INLINE COMMENTS > media.h:95 > + > +friend class MediaPrivate; > +}; Not

D15745: Implement Media and MediaEndpoint API

2018-09-25 Thread David Rosca
drosca added inline comments. INLINE COMMENTS > broulik wrote in media_p.h:35 > I think this needs `Q_DECL_HIDDEN` Why? This class is not exported by default, afaik it is only needed if MediaPrivate was declared inside Media class (eg. Media::MediaPrivate), which it is not. REPOSITORY R269

D18315: Device: Check object path in interfaces removed slot

2019-01-17 Thread David Rosca
drosca created this revision. drosca added a reviewer: Frameworks. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. drosca requested review of this revision. REVISION SUMMARY Only remove Input and MediaPlayer objects when path matches. BUG: 403289 TEST

D17419: Add view-private icon

2018-12-09 Thread David Rosca
drosca added a comment. In D17419#374064 , @ngraham wrote: > Ah yeah, I guess that makes sense. > > #falkon folks, is this acceptable? Yes, looks great! REPOSITORY R266 Breeze Icons

D19805: Manager: Don't require Media1 interface for initialization

2019-03-16 Thread David Rosca
drosca created this revision. drosca added reviewers: Frameworks, broulik. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. drosca requested review of this revision. REVISION SUMMARY BUG: 405478 FIXED-IN: 5.57 REPOSITORY R269 BluezQt BRANCH bug405478

D19805: Manager: Don't require Media1 interface for initialization

2019-03-18 Thread David Rosca
This revision was automatically updated to reflect the committed changes. Closed by commit R269:5da57e174696: Manager: Dont require Media1 interface for initialization (authored by drosca). REPOSITORY R269 BluezQt CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D19805?vs=54013=54187

D18315: Device: Check object path in interfaces removed slot

2019-03-18 Thread David Rosca
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R269:9197a45652be: Device: Check object path in interfaces removed slot (authored by drosca). REPOSITORY R269 BluezQt

D21584: Add LE Advertising and GATT APIs

2019-06-05 Thread David Rosca
drosca requested changes to this revision. drosca added a comment. This revision now requires changes to proceed. Looks really good! INLINE COMMENTS > gattapplication.cpp:38 > + > +GattApplication::GattApplication(QObject *parent) : > +ObjectManager(parent), Coding style:

D21511: Make falkon icon a real SVG

2019-05-31 Thread David Rosca
drosca accepted this revision. This revision is now accepted and ready to land. REPOSITORY R266 Breeze Icons BRANCH falkon (branched from master) REVISION DETAIL https://phabricator.kde.org/D21511 To: ndavis, #vdg, #falkon, abetts, drosca Cc: ngraham, kde-frameworks-devel, michaelh,

D21584: Add LE Advertising and GATT APIs

2019-06-07 Thread David Rosca
drosca requested changes to this revision. drosca added a comment. This revision now requires changes to proceed. After this, it should be good to go. INLINE COMMENTS > drosca wrote in gattapplication.h:47 > Maybe it would be best to add constructor that uses default object path. Also >

D21584: Add LE Advertising and GATT APIs

2019-06-07 Thread David Rosca
drosca accepted this revision. drosca added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > gattapplication.h:85 > + */ > +virtual QDBusObjectPath objectPath() const; > + This should be protected > gattapplication.h:96 > + */ > +

D21584: Add LE Advertising and GATT APIs

2019-06-07 Thread David Rosca
drosca added a comment. In D21584#475914 , @ngraham wrote: > What is the net effect of this patch? What does it improve? It adds initial support for Bluetooth Low Energy, it's not used anywhere in Plasma. REPOSITORY R269 BluezQt

D21584: Add LE Advertising and GATT APIs

2019-06-06 Thread David Rosca
drosca requested changes to this revision. drosca added a comment. This revision now requires changes to proceed. It seems you uploaded only diff with latest changes, you need to upload the entire diff against master. INLINE COMMENTS > gattapplication.h:47 > */ > -explicit

D22107: Add MediaTransport API

2019-06-28 Thread David Rosca
drosca accepted this revision. drosca added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > mweichselbaumer wrote in tpendingcall.h:45 > Actually, my intention was to provide a type-safe way to obtain return values > from PendingCall and to express

D19960: bluez-qt: remove warnings

2019-04-09 Thread David Rosca
drosca accepted this revision. This revision is now accepted and ready to land. REPOSITORY R269 BluezQt BRANCH remove_compile_warnings REVISION DETAIL https://phabricator.kde.org/D19960 To: carneirogustavo, andreagenor, tcanabrava, patrickelectric, drosca Cc: pino, kde-frameworks-devel,

D19960: bluez-qt: remove warnings

2019-04-09 Thread David Rosca
drosca requested changes to this revision. drosca added a comment. This revision now requires changes to proceed. Actually ... INLINE COMMENTS > a2dp-codecs.c:59 > AAC_OBJECT_TYPE_MPEG4_AAC_SCA, */ > -AAC_INIT_FREQUENCY( > +AAC_INIT_FREQUENCY(( > /*AAC_SAMPLING_FREQ_8000 |

D23988: [Bluez-Qt] Port away from deprecated QSharedPointer::data() method.

2019-09-16 Thread David Rosca
drosca accepted this revision. This revision is now accepted and ready to land. REPOSITORY R269 BluezQt BRANCH master REVISION DETAIL https://phabricator.kde.org/D23988 To: dfaure, drosca Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D22107: Add MediaTransport API

2019-06-26 Thread David Rosca
drosca requested changes to this revision. drosca added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > a2dp-codecs.h:33 > #define A2DP_CODEC_MPEG240x02 > -#define A2DP_CODEC_ATRAC 0x03 > +#define A2DP_CODEC_ATRAC 0x04 >

D22346: Should we install bluezqt_dbustypes.h?

2019-07-09 Thread David Rosca
drosca added a comment. Sorry, that slipped in the review. Fixed now: https://cgit.kde.org/bluez-qt.git/commit/?id=185e2c70331f08c61cc6903336eaf25a5193e352 REPOSITORY R269 BluezQt REVISION DETAIL https://phabricator.kde.org/D22346 To: joselema, drosca, mweichselbaumer Cc:

D25859: Add Battery1 interface

2019-12-13 Thread David Rosca
drosca accepted this revision. drosca added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > batterytest.cpp:40 > + > +//qRegisterMetaType("ReconnectMode"); > +} If it's not needed, then just remove it. REPOSITORY R269 BluezQt REVISION DETAIL

D27112: Fix errors in the QRegularExpression porting commit

2020-03-06 Thread David Rosca
drosca accepted this revision. This revision is now accepted and ready to land. REPOSITORY R269 BluezQt BRANCH l-fix (branched from master) REVISION DETAIL https://phabricator.kde.org/D27112 To: ahmadsamir, #frameworks, drosca, apol, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack,

<    1   2