Re: Review Request 115218: rename dbus interface file on install for kwallet

2014-01-27 Thread Jonathan Riddell

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

(Updated Jan. 27, 2014, 4:10 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Valentin Rusu.


Repository: kwallet-framework


Description
---

Rename kwallet dbus interface file on install so it does not clash with 
equivalent file from kdelibs4.  The dbus interface remains the same to keep 
compatibility with kdelibs4 kwallet.

However you seem to have renamed the interface in places in the code in 
runtime/, will it be incompatible with the version from kdelibs4?


Diffs
-

  src/api/KWallet/CMakeLists.txt d0d5a3d 

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


Testing
---


Thanks,

Jonathan Riddell

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


Re: Review Request 115218: rename dbus interface file on install for kwallet

2014-01-27 Thread Commit Hook

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


This review has been submitted with commit 
38913b952073bbeb8784dcd45a267594ccb9057e by Jonathan Riddell to branch master.

- Commit Hook


On Jan. 22, 2014, 11:23 a.m., Jonathan Riddell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/115218/
 ---
 
 (Updated Jan. 22, 2014, 11:23 a.m.)
 
 
 Review request for KDE Frameworks and Valentin Rusu.
 
 
 Repository: kwallet-framework
 
 
 Description
 ---
 
 Rename kwallet dbus interface file on install so it does not clash with 
 equivalent file from kdelibs4.  The dbus interface remains the same to keep 
 compatibility with kdelibs4 kwallet.
 
 However you seem to have renamed the interface in places in the code in 
 runtime/, will it be incompatible with the version from kdelibs4?
 
 
 Diffs
 -
 
   src/api/KWallet/CMakeLists.txt d0d5a3d 
 
 Diff: https://git.reviewboard.kde.org/r/115218/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jonathan Riddell
 


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


Re: Review Request 115218: rename dbus interface file on install for kwallet

2014-01-24 Thread Valentin Rusu

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

Ship it!


Ship It!

- Valentin Rusu


On Jan. 22, 2014, 11:23 a.m., Jonathan Riddell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/115218/
 ---
 
 (Updated Jan. 22, 2014, 11:23 a.m.)
 
 
 Review request for KDE Frameworks and Valentin Rusu.
 
 
 Repository: kwallet-framework
 
 
 Description
 ---
 
 Rename kwallet dbus interface file on install so it does not clash with 
 equivalent file from kdelibs4.  The dbus interface remains the same to keep 
 compatibility with kdelibs4 kwallet.
 
 However you seem to have renamed the interface in places in the code in 
 runtime/, will it be incompatible with the version from kdelibs4?
 
 
 Diffs
 -
 
   src/api/KWallet/CMakeLists.txt d0d5a3d 
 
 Diff: https://git.reviewboard.kde.org/r/115218/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jonathan Riddell
 


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


Re: Review Request 115218: rename dbus interface file on install for kwallet

2014-01-23 Thread Jonathan Riddell


 On Jan. 22, 2014, 9:27 p.m., Valentin Rusu wrote:
  src/api/KWallet/CMakeLists.txt, line 100
  https://git.reviewboard.kde.org/r/115218/diff/1/?file=235142#file235142line100
 
  The interface will be strictly the same. New features will be added to 
  secret service. That's why I didn't consider changing it's name. And, btw, 
  changing it's name will make users think that changes have been (or will 
  be) done to this API.
 
 Hrvoje Senjan wrote:
 @Valentin,
 interface name is now org.kde.kwalletd5, as per your last change in 
 api/KWallet/kwallet.cpp  runtime/kwalletd/kwalletd.cpp.

Yes you changed the interface which you say is unnecessary.  My patch is just a 
change of filename to allow co-installability with kdelibs4 but the dbus 
interface name isn't changed.


- Jonathan


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


On Jan. 22, 2014, 11:23 a.m., Jonathan Riddell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/115218/
 ---
 
 (Updated Jan. 22, 2014, 11:23 a.m.)
 
 
 Review request for KDE Frameworks and Valentin Rusu.
 
 
 Repository: kwallet-framework
 
 
 Description
 ---
 
 Rename kwallet dbus interface file on install so it does not clash with 
 equivalent file from kdelibs4.  The dbus interface remains the same to keep 
 compatibility with kdelibs4 kwallet.
 
 However you seem to have renamed the interface in places in the code in 
 runtime/, will it be incompatible with the version from kdelibs4?
 
 
 Diffs
 -
 
   src/api/KWallet/CMakeLists.txt d0d5a3d 
 
 Diff: https://git.reviewboard.kde.org/r/115218/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jonathan Riddell
 


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


Re: Review Request 115218: rename dbus interface file on install for kwallet

2014-01-23 Thread Valentin Rusu


 On Jan. 22, 2014, 9:27 p.m., Valentin Rusu wrote:
  src/api/KWallet/CMakeLists.txt, line 100
  https://git.reviewboard.kde.org/r/115218/diff/1/?file=235142#file235142line100
 
  The interface will be strictly the same. New features will be added to 
  secret service. That's why I didn't consider changing it's name. And, btw, 
  changing it's name will make users think that changes have been (or will 
  be) done to this API.
 
 Hrvoje Senjan wrote:
 @Valentin,
 interface name is now org.kde.kwalletd5, as per your last change in 
 api/KWallet/kwallet.cpp  runtime/kwalletd/kwalletd.cpp.
 
 Jonathan Riddell wrote:
 Yes you changed the interface which you say is unnecessary.  My patch is 
 just a change of filename to allow co-installability with kdelibs4 but the 
 dbus interface name isn't changed.


Considering that, with KDE4:
kwalletd
|
--- exposes org.kde.kwalletd
 |
 - implements interface org.kde.KWallet.xml

With KF5:
kwalletd5 (binary, must have different name to coinstall in same bin directory)
|
--- exposes org.kde.kwalletd5 (dbus object name, must have different name to be 
able to be launched on the same dbus session)
 |
 - implements interface org.kde.KWallet.xml the same as KDE4, 
installation directory is the same, but if replaced there's no conflict (or am 
I wrong?)

I think that there is no need to do a second copy of og.kde.KWallet.xml
What do you think about this?


- Valentin


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


On Jan. 22, 2014, 11:23 a.m., Jonathan Riddell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/115218/
 ---
 
 (Updated Jan. 22, 2014, 11:23 a.m.)
 
 
 Review request for KDE Frameworks and Valentin Rusu.
 
 
 Repository: kwallet-framework
 
 
 Description
 ---
 
 Rename kwallet dbus interface file on install so it does not clash with 
 equivalent file from kdelibs4.  The dbus interface remains the same to keep 
 compatibility with kdelibs4 kwallet.
 
 However you seem to have renamed the interface in places in the code in 
 runtime/, will it be incompatible with the version from kdelibs4?
 
 
 Diffs
 -
 
   src/api/KWallet/CMakeLists.txt d0d5a3d 
 
 Diff: https://git.reviewboard.kde.org/r/115218/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jonathan Riddell
 


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


Re: Review Request 115218: rename dbus interface file on install for kwallet

2014-01-23 Thread Hrvoje Senjan


 On Jan. 22, 2014, 9:27 p.m., Valentin Rusu wrote:
  src/api/KWallet/CMakeLists.txt, line 100
  https://git.reviewboard.kde.org/r/115218/diff/1/?file=235142#file235142line100
 
  The interface will be strictly the same. New features will be added to 
  secret service. That's why I didn't consider changing it's name. And, btw, 
  changing it's name will make users think that changes have been (or will 
  be) done to this API.
 
 Hrvoje Senjan wrote:
 @Valentin,
 interface name is now org.kde.kwalletd5, as per your last change in 
 api/KWallet/kwallet.cpp  runtime/kwalletd/kwalletd.cpp.
 
 Jonathan Riddell wrote:
 Yes you changed the interface which you say is unnecessary.  My patch is 
 just a change of filename to allow co-installability with kdelibs4 but the 
 dbus interface name isn't changed.

 
 Valentin Rusu wrote:
 Considering that, with KDE4:
 kwalletd
 |
 --- exposes org.kde.kwalletd
  |
  - implements interface org.kde.KWallet.xml
 
 With KF5:
 kwalletd5 (binary, must have different name to coinstall in same bin 
 directory)
 |
 --- exposes org.kde.kwalletd5 (dbus object name, must have different name 
 to be able to be launched on the same dbus session)
  |
  - implements interface org.kde.KWallet.xml the same as KDE4, 
 installation directory is the same, but if replaced there's no conflict (or 
 am I wrong?)
 
 I think that there is no need to do a second copy of og.kde.KWallet.xml
 What do you think about this?


exposes org.kde.kwalletd5 (dbus object name, must have different name to be 
able to be launched on the same dbus session)
ApplicationName still has kwalletd, so two daemons (kwalletd  kwalletd5) atm 
cannot be started. i guess that was just omission in your change? ;-)

but if replaced there's no conflict
how do you mean? we cannot ship 2 packages providing the same filename and not 
make the conflict. that is the point of this request =)
(yes, it could be confusing, but so far we found this to be the 'best' approach 
- if interface is left to be named same as gen 4.x. maybe to add a comment 
above the install() section?)


- Hrvoje


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


On Jan. 22, 2014, 11:23 a.m., Jonathan Riddell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/115218/
 ---
 
 (Updated Jan. 22, 2014, 11:23 a.m.)
 
 
 Review request for KDE Frameworks and Valentin Rusu.
 
 
 Repository: kwallet-framework
 
 
 Description
 ---
 
 Rename kwallet dbus interface file on install so it does not clash with 
 equivalent file from kdelibs4.  The dbus interface remains the same to keep 
 compatibility with kdelibs4 kwallet.
 
 However you seem to have renamed the interface in places in the code in 
 runtime/, will it be incompatible with the version from kdelibs4?
 
 
 Diffs
 -
 
   src/api/KWallet/CMakeLists.txt d0d5a3d 
 
 Diff: https://git.reviewboard.kde.org/r/115218/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jonathan Riddell
 


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


Review Request 115218: rename dbus interface file on install for kwallet

2014-01-22 Thread Jonathan Riddell

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

Review request for KDE Frameworks and Valentin Rusu.


Repository: kwallet-framework


Description
---

Rename kwallet dbus interface file on install so it does not clash with 
equivalent file from kdelibs4.  The dbus interface remains the same to keep 
compatibility with kdelibs4 kwallet.

However you seem to have renamed the interface in places in the code in 
runtime/, will it be incompatible with the version from kdelibs4?


Diffs
-

  src/api/KWallet/CMakeLists.txt d0d5a3d 

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


Testing
---


Thanks,

Jonathan Riddell

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


Re: Review Request 115218: rename dbus interface file on install for kwallet

2014-01-22 Thread Valentin Rusu

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



src/api/KWallet/CMakeLists.txt
https://git.reviewboard.kde.org/r/115218/#comment34033

The interface will be strictly the same. New features will be added to 
secret service. That's why I didn't consider changing it's name. And, btw, 
changing it's name will make users think that changes have been (or will be) 
done to this API.


- Valentin Rusu


On Jan. 22, 2014, 11:23 a.m., Jonathan Riddell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/115218/
 ---
 
 (Updated Jan. 22, 2014, 11:23 a.m.)
 
 
 Review request for KDE Frameworks and Valentin Rusu.
 
 
 Repository: kwallet-framework
 
 
 Description
 ---
 
 Rename kwallet dbus interface file on install so it does not clash with 
 equivalent file from kdelibs4.  The dbus interface remains the same to keep 
 compatibility with kdelibs4 kwallet.
 
 However you seem to have renamed the interface in places in the code in 
 runtime/, will it be incompatible with the version from kdelibs4?
 
 
 Diffs
 -
 
   src/api/KWallet/CMakeLists.txt d0d5a3d 
 
 Diff: https://git.reviewboard.kde.org/r/115218/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jonathan Riddell
 


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


Re: Review Request 115218: rename dbus interface file on install for kwallet

2014-01-22 Thread Hrvoje Senjan


 On Jan. 22, 2014, 9:27 p.m., Valentin Rusu wrote:
  src/api/KWallet/CMakeLists.txt, line 100
  https://git.reviewboard.kde.org/r/115218/diff/1/?file=235142#file235142line100
 
  The interface will be strictly the same. New features will be added to 
  secret service. That's why I didn't consider changing it's name. And, btw, 
  changing it's name will make users think that changes have been (or will 
  be) done to this API.

@Valentin,
interface name is now org.kde.kwalletd5, as per your last change in 
api/KWallet/kwallet.cpp  runtime/kwalletd/kwalletd.cpp.


- Hrvoje


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


On Jan. 22, 2014, 11:23 a.m., Jonathan Riddell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/115218/
 ---
 
 (Updated Jan. 22, 2014, 11:23 a.m.)
 
 
 Review request for KDE Frameworks and Valentin Rusu.
 
 
 Repository: kwallet-framework
 
 
 Description
 ---
 
 Rename kwallet dbus interface file on install so it does not clash with 
 equivalent file from kdelibs4.  The dbus interface remains the same to keep 
 compatibility with kdelibs4 kwallet.
 
 However you seem to have renamed the interface in places in the code in 
 runtime/, will it be incompatible with the version from kdelibs4?
 
 
 Diffs
 -
 
   src/api/KWallet/CMakeLists.txt d0d5a3d 
 
 Diff: https://git.reviewboard.kde.org/r/115218/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jonathan Riddell
 


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