Re: Review Request 121121: Remove all kdelibs4support from polkit-agent

2014-11-17 Thread David Edmundson

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

(Updated Nov. 17, 2014, 2:37 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma, Jaroslav Řezník and Lukáš Tinkl.


Repository: polkit-kde-agent-1


Description
---

Remove all kdelibs4support from polkit-agent


Diffs
-

  AuthDialog.h eaebbd2 
  AuthDialog.cpp b76d91e 
  AuthDialog.ui e4da0f9 
  CMakeLists.txt 482fc57 
  main.cpp f8f97b8 
  policykitkde.h 85ce6b2 
  policykitkde.cpp a25cb4f 
  policykitlistener.h 538381f 
  policykitlistener.cpp bab7fdf 

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


Testing
---

Using pkexec tested using the correct password the wrong password viewing 
details and tested trying to open two agents at once.


File Attachments


Screenshot
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/11/16/1bb7a716-e855-4ad3-9c36-498077ee016b__polkit_normal.png
screenshot --reverse
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/11/16/0e03dc82-a81f-4192-81c7-f6376d545681__polkit_reverse.png


Thanks,

David Edmundson

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


Re: Review Request 121121: Remove all kdelibs4support from polkit-agent

2014-11-16 Thread Lukáš Tinkl

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

Ship it!


Ship It!

- Lukáš Tinkl


On Lis. 16, 2014, 2:16 odp., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121121/
> ---
> 
> (Updated Lis. 16, 2014, 2:16 odp.)
> 
> 
> Review request for Plasma, Jaroslav Řezník and Lukáš Tinkl.
> 
> 
> Repository: polkit-kde-agent-1
> 
> 
> Description
> ---
> 
> Remove all kdelibs4support from polkit-agent
> 
> 
> Diffs
> -
> 
>   AuthDialog.h eaebbd2 
>   AuthDialog.cpp b76d91e 
>   AuthDialog.ui e4da0f9 
>   CMakeLists.txt 482fc57 
>   main.cpp f8f97b8 
>   policykitkde.h 85ce6b2 
>   policykitkde.cpp a25cb4f 
>   policykitlistener.h 538381f 
>   policykitlistener.cpp bab7fdf 
> 
> Diff: https://git.reviewboard.kde.org/r/121121/diff/
> 
> 
> Testing
> ---
> 
> Using pkexec tested using the correct password the wrong password viewing 
> details and tested trying to open two agents at once.
> 
> 
> File Attachments
> 
> 
> Screenshot
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/11/16/1bb7a716-e855-4ad3-9c36-498077ee016b__polkit_normal.png
> screenshot --reverse
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/11/16/0e03dc82-a81f-4192-81c7-f6376d545681__polkit_reverse.png
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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


Re: Review Request 121121: Remove all kdelibs4support from polkit-agent

2014-11-16 Thread David Edmundson

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

(Updated Nov. 16, 2014, 1:16 p.m.)


Review request for Plasma, Jaroslav Řezník and Lukáš Tinkl.


Changes
---

I took screenshots of normal and --reverse with the current code. It seems to 
be correct, as not only is the text reversed but the button is too. So the 
arrows still point towards the inside of the dialog to open.

Given the text is for styling not actual words, I don't think it should be 
added to the string. 

If the screenshots don't convince you, I'll change it :)


Repository: polkit-kde-agent-1


Description
---

Remove all kdelibs4support from polkit-agent


Diffs
-

  AuthDialog.h eaebbd2 
  AuthDialog.cpp b76d91e 
  AuthDialog.ui e4da0f9 
  CMakeLists.txt 482fc57 
  main.cpp f8f97b8 
  policykitkde.h 85ce6b2 
  policykitkde.cpp a25cb4f 
  policykitlistener.h 538381f 
  policykitlistener.cpp bab7fdf 

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


Testing
---

Using pkexec tested using the correct password the wrong password viewing 
details and tested trying to open two agents at once.


File Attachments (updated)


Screenshot
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/11/16/1bb7a716-e855-4ad3-9c36-498077ee016b__polkit_normal.png
screenshot --reverse
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/11/16/0e03dc82-a81f-4192-81c7-f6376d545681__polkit_reverse.png


Thanks,

David Edmundson

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


Re: Review Request 121121: Remove all kdelibs4support from polkit-agent

2014-11-16 Thread Lukáš Tinkl


> On Lis. 14, 2014, 6:27 odp., Martin Klapetek wrote:
> > AuthDialog.cpp, lines 75-76
> > 
> >
> > Shouldn't this be one i18n("Details >>") to avoid string puzzles?
> > 
> > Would require another "Details <<" though, but that's necessary for RTL 
> > languages anyway (no?)
> 
> David Edmundson wrote:
> KDialog does it this way.
> 
> Martin Klapetek wrote:
> That doesn't make it correct.
> 
> 
> http://api.kde.org/frameworks-api/frameworks5-apidocs/ki18n/html/prg_guide.html#good_text

Include it in the string, I fancy RTL languages might want to put it on the 
left side


- Lukáš


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


On Lis. 14, 2014, 6:08 odp., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121121/
> ---
> 
> (Updated Lis. 14, 2014, 6:08 odp.)
> 
> 
> Review request for Plasma, Jaroslav Řezník and Lukáš Tinkl.
> 
> 
> Repository: polkit-kde-agent-1
> 
> 
> Description
> ---
> 
> Remove all kdelibs4support from polkit-agent
> 
> 
> Diffs
> -
> 
>   AuthDialog.h eaebbd2 
>   AuthDialog.cpp b76d91e 
>   AuthDialog.ui e4da0f9 
>   CMakeLists.txt 482fc57 
>   main.cpp f8f97b8 
>   policykitkde.h 85ce6b2 
>   policykitkde.cpp a25cb4f 
>   policykitlistener.h 538381f 
>   policykitlistener.cpp bab7fdf 
> 
> Diff: https://git.reviewboard.kde.org/r/121121/diff/
> 
> 
> Testing
> ---
> 
> Using pkexec tested using the correct password the wrong password viewing 
> details and tested trying to open two agents at once.
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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


Re: Review Request 121121: Remove all kdelibs4support from polkit-agent

2014-11-14 Thread Martin Klapetek


> On Nov. 14, 2014, 6:27 p.m., Martin Klapetek wrote:
> > AuthDialog.cpp, lines 75-76
> > 
> >
> > Shouldn't this be one i18n("Details >>") to avoid string puzzles?
> > 
> > Would require another "Details <<" though, but that's necessary for RTL 
> > languages anyway (no?)
> 
> David Edmundson wrote:
> KDialog does it this way.

That doesn't make it correct.

http://api.kde.org/frameworks-api/frameworks5-apidocs/ki18n/html/prg_guide.html#good_text


- Martin


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


On Nov. 14, 2014, 6:08 p.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121121/
> ---
> 
> (Updated Nov. 14, 2014, 6:08 p.m.)
> 
> 
> Review request for Plasma, Jaroslav Řezník and Lukáš Tinkl.
> 
> 
> Repository: polkit-kde-agent-1
> 
> 
> Description
> ---
> 
> Remove all kdelibs4support from polkit-agent
> 
> 
> Diffs
> -
> 
>   AuthDialog.h eaebbd2 
>   AuthDialog.cpp b76d91e 
>   AuthDialog.ui e4da0f9 
>   CMakeLists.txt 482fc57 
>   main.cpp f8f97b8 
>   policykitkde.h 85ce6b2 
>   policykitkde.cpp a25cb4f 
>   policykitlistener.h 538381f 
>   policykitlistener.cpp bab7fdf 
> 
> Diff: https://git.reviewboard.kde.org/r/121121/diff/
> 
> 
> Testing
> ---
> 
> Using pkexec tested using the correct password the wrong password viewing 
> details and tested trying to open two agents at once.
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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


Re: Review Request 121121: Remove all kdelibs4support from polkit-agent

2014-11-14 Thread David Edmundson


> On Nov. 14, 2014, 5:27 p.m., Martin Klapetek wrote:
> > AuthDialog.cpp, lines 75-76
> > 
> >
> > Shouldn't this be one i18n("Details >>") to avoid string puzzles?
> > 
> > Would require another "Details <<" though, but that's necessary for RTL 
> > languages anyway (no?)

KDialog does it this way.


- David


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


On Nov. 14, 2014, 5:08 p.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121121/
> ---
> 
> (Updated Nov. 14, 2014, 5:08 p.m.)
> 
> 
> Review request for Plasma, Jaroslav Řezník and Lukáš Tinkl.
> 
> 
> Repository: polkit-kde-agent-1
> 
> 
> Description
> ---
> 
> Remove all kdelibs4support from polkit-agent
> 
> 
> Diffs
> -
> 
>   AuthDialog.h eaebbd2 
>   AuthDialog.cpp b76d91e 
>   AuthDialog.ui e4da0f9 
>   CMakeLists.txt 482fc57 
>   main.cpp f8f97b8 
>   policykitkde.h 85ce6b2 
>   policykitkde.cpp a25cb4f 
>   policykitlistener.h 538381f 
>   policykitlistener.cpp bab7fdf 
> 
> Diff: https://git.reviewboard.kde.org/r/121121/diff/
> 
> 
> Testing
> ---
> 
> Using pkexec tested using the correct password the wrong password viewing 
> details and tested trying to open two agents at once.
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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


Re: Review Request 121121: Remove all kdelibs4support from polkit-agent

2014-11-14 Thread Martin Klapetek

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



AuthDialog.cpp


Shouldn't this be one i18n("Details >>") to avoid string puzzles?

Would require another "Details <<" though, but that's necessary for RTL 
languages anyway (no?)


- Martin Klapetek


On Nov. 14, 2014, 6:08 p.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121121/
> ---
> 
> (Updated Nov. 14, 2014, 6:08 p.m.)
> 
> 
> Review request for Plasma, Jaroslav Řezník and Lukáš Tinkl.
> 
> 
> Repository: polkit-kde-agent-1
> 
> 
> Description
> ---
> 
> Remove all kdelibs4support from polkit-agent
> 
> 
> Diffs
> -
> 
>   AuthDialog.h eaebbd2 
>   AuthDialog.cpp b76d91e 
>   AuthDialog.ui e4da0f9 
>   CMakeLists.txt 482fc57 
>   main.cpp f8f97b8 
>   policykitkde.h 85ce6b2 
>   policykitkde.cpp a25cb4f 
>   policykitlistener.h 538381f 
>   policykitlistener.cpp bab7fdf 
> 
> Diff: https://git.reviewboard.kde.org/r/121121/diff/
> 
> 
> Testing
> ---
> 
> Using pkexec tested using the correct password the wrong password viewing 
> details and tested trying to open two agents at once.
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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


Re: Review Request 121121: Remove all kdelibs4support from polkit-agent

2014-11-14 Thread Aleix Pol Gonzalez

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


+1, looks good, maintainers will know best.

- Aleix Pol Gonzalez


On Nov. 14, 2014, 5:08 p.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121121/
> ---
> 
> (Updated Nov. 14, 2014, 5:08 p.m.)
> 
> 
> Review request for Plasma, Jaroslav Řezník and Lukáš Tinkl.
> 
> 
> Repository: polkit-kde-agent-1
> 
> 
> Description
> ---
> 
> Remove all kdelibs4support from polkit-agent
> 
> 
> Diffs
> -
> 
>   AuthDialog.h eaebbd2 
>   AuthDialog.cpp b76d91e 
>   AuthDialog.ui e4da0f9 
>   CMakeLists.txt 482fc57 
>   main.cpp f8f97b8 
>   policykitkde.h 85ce6b2 
>   policykitkde.cpp a25cb4f 
>   policykitlistener.h 538381f 
>   policykitlistener.cpp bab7fdf 
> 
> Diff: https://git.reviewboard.kde.org/r/121121/diff/
> 
> 
> Testing
> ---
> 
> Using pkexec tested using the correct password the wrong password viewing 
> details and tested trying to open two agents at once.
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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


Review Request 121121: Remove all kdelibs4support from polkit-agent

2014-11-14 Thread David Edmundson

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

Review request for Plasma, Jaroslav Řezník and Lukáš Tinkl.


Repository: polkit-kde-agent-1


Description
---

Remove all kdelibs4support from polkit-agent


Diffs
-

  AuthDialog.h eaebbd2 
  AuthDialog.cpp b76d91e 
  AuthDialog.ui e4da0f9 
  CMakeLists.txt 482fc57 
  main.cpp f8f97b8 
  policykitkde.h 85ce6b2 
  policykitkde.cpp a25cb4f 
  policykitlistener.h 538381f 
  policykitlistener.cpp bab7fdf 

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


Testing
---

Using pkexec tested using the correct password the wrong password viewing 
details and tested trying to open two agents at once.


Thanks,

David Edmundson

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