Re: Review Request 123360: Introduce LoopSound flag

2015-04-15 Thread Martin Klapetek 

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

(Updated April 15, 2015, 11:21 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit 5281dbec2188c29f3e236f54cbd24fd26d171365 by Martin 
Klapetek to branch master.


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


Repository: knotifications


Description
---

Some notifications might want to loop the sound, eg. calling apps playing the 
ringing sound which is not wanted to play once. Currently the sound is looped 
for all persistent notifications, but that's not ideal as not all persistent 
notifications with sound want/need sound looping. This new LoopSound flag 
solves that.


Diffs
-

  src/knotification.h 8efd92a 
  src/notifybyaudio.cpp ee3ef1e 

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


Testing
---


Thanks,

Martin Klapetek

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 123360: Introduce LoopSound flag

2015-04-14 Thread Lamarque Souza 

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

Ship it!


Ship It!

- Lamarque Souza


On April 14, 2015, 2:41 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123360/
> ---
> 
> (Updated April 14, 2015, 2:41 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 346148
> https://bugs.kde.org/show_bug.cgi?id=346148
> 
> 
> Repository: knotifications
> 
> 
> Description
> ---
> 
> Some notifications might want to loop the sound, eg. calling apps playing the 
> ringing sound which is not wanted to play once. Currently the sound is looped 
> for all persistent notifications, but that's not ideal as not all persistent 
> notifications with sound want/need sound looping. This new LoopSound flag 
> solves that.
> 
> 
> Diffs
> -
> 
>   src/knotification.h 8efd92a 
>   src/notifybyaudio.cpp ee3ef1e 
> 
> Diff: https://git.reviewboard.kde.org/r/123360/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 123360: Introduce LoopSound flag

2015-04-14 Thread Lamarque Souza 


> On April 14, 2015, 3:50 p.m., Lamarque Souza wrote:
> > src/notifybyaudio.cpp, line 89
> > 
> >
> > Should not this patch be followed by another patch that set LoopSound 
> > flag for Persistant notifications that *really* want sound to loop? This 
> > change can be considered a regression if such a persistant notification 
> > exists.
> 
> Martin Klapetek wrote:
> I'm not aware of any such notification (also note that this code was 
> written from scratch for frameworks only). The only place I know I will use 
> it is ktp-call-ui but I really don't see why should that patch be posted here 
> as a follow-up...

That is fair, no more issues from my side then.


- Lamarque


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


On April 14, 2015, 2:41 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123360/
> ---
> 
> (Updated April 14, 2015, 2:41 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 346148
> https://bugs.kde.org/show_bug.cgi?id=346148
> 
> 
> Repository: knotifications
> 
> 
> Description
> ---
> 
> Some notifications might want to loop the sound, eg. calling apps playing the 
> ringing sound which is not wanted to play once. Currently the sound is looped 
> for all persistent notifications, but that's not ideal as not all persistent 
> notifications with sound want/need sound looping. This new LoopSound flag 
> solves that.
> 
> 
> Diffs
> -
> 
>   src/knotification.h 8efd92a 
>   src/notifybyaudio.cpp ee3ef1e 
> 
> Diff: https://git.reviewboard.kde.org/r/123360/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 123360: Introduce LoopSound flag

2015-04-14 Thread Martin Klapetek 


> On April 14, 2015, 5:50 p.m., Lamarque Souza wrote:
> > src/notifybyaudio.cpp, line 89
> > 
> >
> > Should not this patch be followed by another patch that set LoopSound 
> > flag for Persistant notifications that *really* want sound to loop? This 
> > change can be considered a regression if such a persistant notification 
> > exists.

I'm not aware of any such notification (also note that this code was written 
from scratch for frameworks only). The only place I know I will use it is 
ktp-call-ui but I really don't see why should that patch be posted here as a 
follow-up...


- Martin


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


On April 14, 2015, 4:41 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123360/
> ---
> 
> (Updated April 14, 2015, 4:41 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 346148
> https://bugs.kde.org/show_bug.cgi?id=346148
> 
> 
> Repository: knotifications
> 
> 
> Description
> ---
> 
> Some notifications might want to loop the sound, eg. calling apps playing the 
> ringing sound which is not wanted to play once. Currently the sound is looped 
> for all persistent notifications, but that's not ideal as not all persistent 
> notifications with sound want/need sound looping. This new LoopSound flag 
> solves that.
> 
> 
> Diffs
> -
> 
>   src/knotification.h 8efd92a 
>   src/notifybyaudio.cpp ee3ef1e 
> 
> Diff: https://git.reviewboard.kde.org/r/123360/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 123360: Introduce LoopSound flag

2015-04-14 Thread Lamarque Souza 

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



src/knotification.h (line 3)


Update copyright date.



src/notifybyaudio.cpp (line 2)


Update copyright date.



src/notifybyaudio.cpp (line 89)


Should not this patch be followed by another patch that set LoopSound flag 
for Persistant notifications that *really* want sound to loop? This change can 
be considered a regression if such a persistant notification exists.


- Lamarque Souza


On April 14, 2015, 2:41 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123360/
> ---
> 
> (Updated April 14, 2015, 2:41 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 346148
> https://bugs.kde.org/show_bug.cgi?id=346148
> 
> 
> Repository: knotifications
> 
> 
> Description
> ---
> 
> Some notifications might want to loop the sound, eg. calling apps playing the 
> ringing sound which is not wanted to play once. Currently the sound is looped 
> for all persistent notifications, but that's not ideal as not all persistent 
> notifications with sound want/need sound looping. This new LoopSound flag 
> solves that.
> 
> 
> Diffs
> -
> 
>   src/knotification.h 8efd92a 
>   src/notifybyaudio.cpp ee3ef1e 
> 
> Diff: https://git.reviewboard.kde.org/r/123360/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 123360: Introduce LoopSound flag

2015-04-14 Thread Martin Klapetek 

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

Review request for KDE Frameworks.


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


Repository: knotifications


Description
---

Some notifications might want to loop the sound, eg. calling apps playing the 
ringing sound which is not wanted to play once. Currently the sound is looped 
for all persistent notifications, but that's not ideal as not all persistent 
notifications with sound want/need sound looping. This new LoopSound flag 
solves that.


Diffs
-

  src/knotification.h 8efd92a 
  src/notifybyaudio.cpp ee3ef1e 

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


Testing
---


Thanks,

Martin Klapetek

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel