Re: [PATCH] [kcms/phonon] Don't overwrite audio profile entries with same priority

[Bernie Innocenti]

On 15/06/15 22:01, Sebastian Kügler wrote:

Easily: https://identity.kde.org/index.php?r=registration/index

>
> [...]
>

Ah, thanks for the note. I've updated the documentation about sending patches
on Techbase.


Awesome, thanks for getting me started, I posted the review.

A few more suggestions to help streamline the new contributor onboarding 
process:


 - Add a note about reviewboard in HACKING, that's where I looked first

 - Move the "Reviewboard" paragraph near the top of 
https://techbase.kde.org/Contribute/Send_Patches


 - Add a link from https://techbase.kde.org/Development/Review_Board to 
https://techbase.kde.org/Contribute/Get_a_Contributor_Account#Apply_for_an_account


 - The paragraph "Who Can Apply For a KDE Contributor Account?" led me 
to believe that a casual contribution did not qualify for an account. 
Now I realize that I was confusing the basic identity with a full 
developer account, but it would be good to clarify it near the top of 
the page.


 - The Reviewboard page talks about a cli tool called "post-review", 
but I had to use "rbt post" on Fedora (the rpm is called RBTools, like 
on Suse).


--
 _ // Bernie Innocenti
 \X/  http://codewiz.org
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 124108: [kcms/phonon] Don't overwrite audio profile entries with same priority

[Bernie Innocenti]

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

Review request for Plasma and Harald Sitter.


Repository: plasma-desktop


Description
---

My system has 3 HDMI ports, and HDMI2 is listed in pavucontrol,
but not in kcm_phonon. The reason was that HDMI2 and HDMI3 had the
same priority, and were overwriting each other in the cardInfo profiles
map. The fix consists in switching to a multimap.


Diffs
-

  kcms/phonon/audiosetup.h dfb25bf8c111d72065a27bb91583026093f0598d 

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


Testing
---

Manually tested with:
  QT_PLUGIN_PATH="$PWD:$QT_PLUGIN_PATH" kcmshell5 kcm_phonon


Thanks,

Bernie Innocenti

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: [PATCH] [kcms/phonon] Don't overwrite audio profile entries with same priority

[Sebastian Kügler]

On Monday, June 15, 2015 21:25:56 Bernie Innocenti wrote:
> On 15/06/15 13:49, Sebastian Kügler wrote:
> > On Monday, June 15, 2015 04:35:50 Bernie Innocenti wrote:
> >> This is a bug that's been annoying me for ages. My system has 3 HDMI
> >> ports, and HDMI2 would show in pavucontrol, but not show in kcm_phonon.
> >> The reason was that HDMI3 had the same priority of HDMI2 and would be
> >> overridden in the cardInfo profiles QMap.
> > 
> > Could you post the patch to reviewboard and include "sitter" (Harald
> > Sitter) in the reviewers field. Patches on mailinglist are hard to
> > review, hard to track and tend to get lost.
> 
> Sure, but how do I get an account for reviewboard.kde.org? I'm not a 
> regular KDE contributor.

Easily: https://identity.kde.org/index.php?r=registration/index

> On a related note, following the links from the KDE website I landed on 
> this page which appears to be out of date because it still refers to 
> SVN: https://techbase.kde.org/Contribute/Send_Patches . Now I noticed 
> the Reviewboard paragraph at the bottom.

Ah, thanks for the note. I've updated the documentation about sending patches 
on Techbase.

Cheers,
-- 
sebas

Sebastian Kügler|http://vizZzion.org| http://kde.org

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 124102: Replace other Notifications services when Plasma's notifications are enabled

[Martin Klapetek]

> On June 15, 2015, 9:29 p.m., Lukáš Tinkl wrote:
> > A better option would be to call registerService() with 
> > QDBusConnectionInterface::ReplaceExistingService and 
> > DBusConnectionInterface::AllowReplacement

It's the "attempt to replace it." part of it that I'm not entirely happy with. 
Plus we don't actually get to control all the servers and set 
"AllowReplacement" on them, so this might not be feasible in the end.


- Martin


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


On June 15, 2015, 4:30 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124102/
> ---
> 
> (Updated June 15, 2015, 4:30 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> We're getting lots of reports about notifications not being "closeable on 
> click" or "not having any actions" or "not having Plasma theme". These all 
> mostly come from users which have notify-osd package from Unity (ie. Ubuntu 
> users installing plasma-desktop), but not only.
> 
> So this patch makes Plasma always be the Notification service provider if 
> that option is enabled in the applet settings and/or if the applet is present 
> somewhere (otherwise the dataengine is not loaded). On startup, it will get 
> the PID of the current Notifications service, send SIGTERM to it and register 
> its own service.
> 
> 
> Diffs
> -
> 
>   dataengines/notifications/notificationsengine.h 7810787 
>   dataengines/notifications/notificationsengine.cpp c3bf373 
> 
> Diff: https://git.reviewboard.kde.org/r/124102/diff/
> 
> 
> Testing
> ---
> 
> Having notify-osd running, plasmashell starts up, notify-osd is terminated, 
> Plasma notifications appear.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 124102: Replace other Notifications services when Plasma's notifications are enabled

[Lukáš Tinkl]

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


A better option would be to call registerService() with 
QDBusConnectionInterface::ReplaceExistingService and 
DBusConnectionInterface::AllowReplacement

- Lukáš Tinkl


On Čer. 15, 2015, 4:30 odp., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124102/
> ---
> 
> (Updated Čer. 15, 2015, 4:30 odp.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> We're getting lots of reports about notifications not being "closeable on 
> click" or "not having any actions" or "not having Plasma theme". These all 
> mostly come from users which have notify-osd package from Unity (ie. Ubuntu 
> users installing plasma-desktop), but not only.
> 
> So this patch makes Plasma always be the Notification service provider if 
> that option is enabled in the applet settings and/or if the applet is present 
> somewhere (otherwise the dataengine is not loaded). On startup, it will get 
> the PID of the current Notifications service, send SIGTERM to it and register 
> its own service.
> 
> 
> Diffs
> -
> 
>   dataengines/notifications/notificationsengine.h 7810787 
>   dataengines/notifications/notificationsengine.cpp c3bf373 
> 
> Diff: https://git.reviewboard.kde.org/r/124102/diff/
> 
> 
> Testing
> ---
> 
> Having notify-osd running, plasmashell starts up, notify-osd is terminated, 
> Plasma notifications appear.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 124102: Replace other Notifications services when Plasma's notifications are enabled

[Kai Uwe Broulik]

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


+1

*evil laughing up my sleeve*


dataengines/notifications/notificationsengine.cpp (line 63)


Given we require Qt 5.4, QTimer::singleShot(3000, this, 
&NotificationsEngine::registerDBusService);


- Kai Uwe Broulik


On Juni 15, 2015, 2:30 nachm., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124102/
> ---
> 
> (Updated Juni 15, 2015, 2:30 nachm.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> We're getting lots of reports about notifications not being "closeable on 
> click" or "not having any actions" or "not having Plasma theme". These all 
> mostly come from users which have notify-osd package from Unity (ie. Ubuntu 
> users installing plasma-desktop), but not only.
> 
> So this patch makes Plasma always be the Notification service provider if 
> that option is enabled in the applet settings and/or if the applet is present 
> somewhere (otherwise the dataengine is not loaded). On startup, it will get 
> the PID of the current Notifications service, send SIGTERM to it and register 
> its own service.
> 
> 
> Diffs
> -
> 
>   dataengines/notifications/notificationsengine.h 7810787 
>   dataengines/notifications/notificationsengine.cpp c3bf373 
> 
> Diff: https://git.reviewboard.kde.org/r/124102/diff/
> 
> 
> Testing
> ---
> 
> Having notify-osd running, plasmashell starts up, notify-osd is terminated, 
> Plasma notifications appear.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: [PATCH] [kcms/phonon] Don't overwrite audio profile entries with same priority

[Sebastian Kügler]

Hi Bernie,

Thanks for your patch!

On Monday, June 15, 2015 04:35:50 Bernie Innocenti wrote:
> This is a bug that's been annoying me for ages. My system has 3 HDMI
> ports, and HDMI2 would show in pavucontrol, but not show in kcm_phonon.
> The reason was that HDMI3 had the same priority of HDMI2 and would be
> overridden in the cardInfo profiles QMap.

Could you post the patch to reviewboard and include "sitter" (Harald Sitter) 
in the reviewers field. Patches on mailinglist are hard to review, hard to 
track and tend to get lost.

Thanks,
-- 
sebas

Sebastian Kügler|http://vizZzion.org| http://kde.org

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 124093: Bug 345405 - Prevent two wallpaper images showing overlayed when "Scaled, Keep Proportions" is chosen

[David Edmundson]

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

Ship it!


I'm still not super convinced we're not just working round a bug, but I 
couldn't make a test case / fix it any other way after a solid hour of trying.

Effect still looks good, if not better so I'm OK shipping this. I'll merge it 
this evening.

- David Edmundson


On June 15, 2015, 3:52 p.m., William Lieurance wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124093/
> ---
> 
> (Updated June 15, 2015, 3:52 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 345405
> https://bugs.kde.org/show_bug.cgi?id=345405
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Moving the opacity change from a ScriptAction to a parallel OpacityAnimator.  
> That seems to cause a repaint in whatever weird environment happens to be the 
> root of this bug.
> 
> 
> Diffs
> -
> 
>   wallpapers/image/imagepackage/contents/ui/main.qml 
> 23d23ff7003488d74ece0d70a1c68d129f282c75 
> 
> Diff: https://git.reviewboard.kde.org/r/124093/diff/
> 
> 
> Testing
> ---
> 
> See conversation on the bug page 
> (https://bugs.kde.org/show_bug.cgi?id=345405).  This code should work 
> identically to the current script, but for whatever reason it causes a 
> repaint to be issued correctly whereas the other does not.
> 
> 
> Thanks,
> 
> William Lieurance
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 124093: Bug 345405 - Prevent two wallpaper images showing overlayed when "Scaled, Keep Proportions" is chosen

[William Lieurance]

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

(Updated June 15, 2015, 3:52 p.m.)


Review request for Plasma.


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


Repository: plasma-workspace


Description
---

Moving the opacity change from a ScriptAction to a parallel OpacityAnimator.  
That seems to cause a repaint in whatever weird environment happens to be the 
root of this bug.


Diffs (updated)
-

  wallpapers/image/imagepackage/contents/ui/main.qml 
23d23ff7003488d74ece0d70a1c68d129f282c75 

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


Testing
---

See conversation on the bug page (https://bugs.kde.org/show_bug.cgi?id=345405). 
 This code should work identically to the current script, but for whatever 
reason it causes a repaint to be issued correctly whereas the other does not.


Thanks,

William Lieurance

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 124072: Refactor the Calendar grid computation a bit

[Martin Klapetek]

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

(Updated June 15, 2015, 2:35 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Changes
---

Submitted with commit 23add5d6e4c19d353b7d8bdaf9ea2f126e7f30b9 by Martin 
Klapetek to branch master.


Repository: plasma-framework


Description
---

Makes the code a bit simpler & lighter and fixes the sometimes missing bottom 
line in calendar.

The grid is now equally padded from both sides (including the month name), so 
basically it's now
always aligned to the center and this also fixes the cases where there was a 
bigger padding on
one side than on the other.

The grid is now also anchored to the bottom so that the bottom margin can stay 
moreless consistent
with the side margins in different sizes.


Diffs
-

  src/declarativeimports/calendar/qml/DaysCalendar.qml 5308708 
  src/declarativeimports/calendar/qml/MonthView.qml 86e37d8 

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


Testing
---

Lots of 600x-zoom inspections and pixel counting. Tested both in Calendar 
applet and Digital Clock applet.


Thanks,

Martin Klapetek

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 124102: Replace other Notifications services when Plasma's notifications are enabled

[Martin Klapetek]

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

Review request for Plasma.


Repository: plasma-workspace


Description
---

We're getting lots of reports about notifications not being "closeable on 
click" or "not having any actions" or "not having Plasma theme". These all 
mostly come from users which have notify-osd package from Unity (ie. Ubuntu 
users installing plasma-desktop), but not only.

So this patch makes Plasma always be the Notification service provider if that 
option is enabled in the applet settings and/or if the applet is present 
somewhere (otherwise the dataengine is not loaded). On startup, it will get the 
PID of the current Notifications service, send SIGTERM to it and register its 
own service.


Diffs
-

  dataengines/notifications/notificationsengine.h 7810787 
  dataengines/notifications/notificationsengine.cpp c3bf373 

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


Testing
---

Having notify-osd running, plasmashell starts up, notify-osd is terminated, 
Plasma notifications appear.


Thanks,

Martin Klapetek

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 123991: Select lockscreenpassword not until user can type

[David Kahles]

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

(Updated June 15, 2015, 12:07 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Changes
---

Submitted with commit 044f1bcce8bb769a41d3aa4f38aee9b64e608d30 by David Kahles 
to branch master.


Repository: plasma-workspace


Description
---

This selects the (wrong) password at the lockscreen not before the grace lock 
timer is expired, because the user can't type before.


Diffs
-

  lookandfeel/contents/lockscreen/LockScreen.qml 
d5cd081830518ec4d75e675281a5a0bd1cbb59b6 

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


Testing
---

Works fine for me, tested on two archlinux machines.


Thanks,

David Kahles

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 124093: Bug 345405 - Prevent two wallpaper images showing overlayed when "Scaled, Keep Proportions" is chosen

[Kai Uwe Broulik]

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


Looks good to me but someone else must approve.


wallpapers/image/imagepackage/contents/ui/main.qml (line 181)


Please use 4 spaces rather than a tab


- Kai Uwe Broulik


On Juni 15, 2015, 5:47 vorm., William Lieurance wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124093/
> ---
> 
> (Updated Juni 15, 2015, 5:47 vorm.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 345405
> https://bugs.kde.org/show_bug.cgi?id=345405
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Moving the opacity change from a ScriptAction to a parallel OpacityAnimator.  
> That seems to cause a repaint in whatever weird environment happens to be the 
> root of this bug.
> 
> 
> Diffs
> -
> 
>   wallpapers/image/imagepackage/contents/ui/main.qml 
> 23d23ff7003488d74ece0d70a1c68d129f282c75 
> 
> Diff: https://git.reviewboard.kde.org/r/124093/diff/
> 
> 
> Testing
> ---
> 
> See conversation on the bug page 
> (https://bugs.kde.org/show_bug.cgi?id=345405).  This code should work 
> identically to the current script, but for whatever reason it causes a 
> repaint to be issued correctly whereas the other does not.
> 
> 
> Thanks,
> 
> William Lieurance
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

[PATCH] [kcms/phonon] Don't overwrite audio profile entries with same priority

[Bernie Innocenti]

This is a bug that's been annoying me for ages. My system has 3 HDMI
ports, and HDMI2 would show in pavucontrol, but not show in kcm_phonon.
The reason was that HDMI3 had the same priority of HDMI2 and would be
overridden in the cardInfo profiles QMap.
---
 kcms/phonon/audiosetup.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kcms/phonon/audiosetup.h b/kcms/phonon/audiosetup.h
index dfb25bf..72fb2be 100644
--- a/kcms/phonon/audiosetup.h
+++ b/kcms/phonon/audiosetup.h
@@ -33,7 +33,7 @@ typedef struct {
 quint32 index;
 QString name;
 QString icon;
-QMap > profiles;
+QMultiMap > profiles;
 QString activeProfile;
 } cardInfo;
 
-- 
2.4.3

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel