Re: Review Request 128847: [ktp-common-internals] [debugger] Split logic and UI

2016-09-20 Thread Alexandr Akulich 

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

(Updated Sept. 20, 2016, 1:06 p.m.)


Status
--

This change has been marked as submitted.


Review request for Telepathy.


Changes
---

Submitted with commit 33af28091656d4b39412c1a0064b83f28e3064ac by Alexandr 
Akulich to branch master.


Repository: ktp-common-internals


Description
---

The main goal of this change is to split logic and UI parts

This is the first step in direction to debugger, which:
1) works with any Telepathy process with DebugInterface support;
2) detects new processess "on fly";
3) has no hardcoded services;
4) shows one process just once, independently of number of dbus services, 
registered by the process.

The change also opens a way to a QML-based UI at some point in future.

Questionable thing is the "TelepathyProcess" class name.
TelepathyService does not fit, because:
1) Single process can expose a number of services (e.g. MissionControl),
2) The debug interface is applicable to any telepathy application, including 
clients, so word "Service" (which is not associated with clients) would mislead.


I uploaded a draft of "second step" to my scratch repo:
https://quickgit.kde.org/?p=scratch%2Fakulichalexandr%2Fktp-common-internals.git&a=commitdiff&h=7e07b65f330d85527c9a6b014154527f7e3e7c01&hp=db202a7143be88db37e056913a88992fe7ce507d

I will make a ReviewRequest with the second part on this (split) commit landed.


Diffs
-

  tools/debugger/CMakeLists.txt e35de89 
  tools/debugger/debug-message-view.h ae745db 
  tools/debugger/debug-message-view.cpp ea09d79 
  tools/debugger/main-window.cpp 490f803 
  tools/debugger/telepathy-process.h PRE-CREATION 
  tools/debugger/telepathy-process.cpp PRE-CREATION 

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


Testing
---

Works as previously.


Thanks,

Alexandr Akulich

Re: Review Request 128847: [ktp-common-internals] [debugger] Split logic and UI

2016-09-20 Thread Aleix Pol Gonzalez 

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



I'd say that if these changes fit you, they should go in. I honestly never used 
ktp-debugger for more than 1 minute straight.

- Aleix Pol Gonzalez


On Sept. 6, 2016, 4:54 p.m., Alexandr Akulich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128847/
> ---
> 
> (Updated Sept. 6, 2016, 4:54 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-common-internals
> 
> 
> Description
> ---
> 
> The main goal of this change is to split logic and UI parts
> 
> This is the first step in direction to debugger, which:
> 1) works with any Telepathy process with DebugInterface support;
> 2) detects new processess "on fly";
> 3) has no hardcoded services;
> 4) shows one process just once, independently of number of dbus services, 
> registered by the process.
> 
> The change also opens a way to a QML-based UI at some point in future.
> 
> Questionable thing is the "TelepathyProcess" class name.
> TelepathyService does not fit, because:
> 1) Single process can expose a number of services (e.g. MissionControl),
> 2) The debug interface is applicable to any telepathy application, including 
> clients, so word "Service" (which is not associated with clients) would 
> mislead.
> 
> 
> I uploaded a draft of "second step" to my scratch repo:
> https://quickgit.kde.org/?p=scratch%2Fakulichalexandr%2Fktp-common-internals.git&a=commitdiff&h=7e07b65f330d85527c9a6b014154527f7e3e7c01&hp=db202a7143be88db37e056913a88992fe7ce507d
> 
> I will make a ReviewRequest with the second part on this (split) commit 
> landed.
> 
> 
> Diffs
> -
> 
>   tools/debugger/CMakeLists.txt e35de89 
>   tools/debugger/debug-message-view.h ae745db 
>   tools/debugger/debug-message-view.cpp ea09d79 
>   tools/debugger/main-window.cpp 490f803 
>   tools/debugger/telepathy-process.h PRE-CREATION 
>   tools/debugger/telepathy-process.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/128847/diff/
> 
> 
> Testing
> ---
> 
> Works as previously.
> 
> 
> Thanks,
> 
> Alexandr Akulich
> 
>

Re: Review Request 128847: [ktp-common-internals] [debugger] Split logic and UI

2016-09-06 Thread Alexandr Akulich 

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

(Updated Sept. 6, 2016, 7:54 p.m.)


Review request for Telepathy.


Changes
---

Added a link to draft of the next commit


Repository: ktp-common-internals


Description (updated)
---

The main goal of this change is to split logic and UI parts

This is the first step in direction to debugger, which:
1) works with any Telepathy process with DebugInterface support;
2) detects new processess "on fly";
3) has no hardcoded services;
4) shows one process just once, independently of number of dbus services, 
registered by the process.

The change also opens a way to a QML-based UI at some point in future.

Questionable thing is the "TelepathyProcess" class name.
TelepathyService does not fit, because:
1) Single process can expose a number of services (e.g. MissionControl),
2) The debug interface is applicable to any telepathy application, including 
clients, so word "Service" (which is not associated with clients) would mislead.


I uploaded a draft of "second step" to my scratch repo:
https://quickgit.kde.org/?p=scratch%2Fakulichalexandr%2Fktp-common-internals.git&a=commitdiff&h=7e07b65f330d85527c9a6b014154527f7e3e7c01&hp=db202a7143be88db37e056913a88992fe7ce507d

I will make a ReviewRequest with the second part on this (split) commit landed.


Diffs
-

  tools/debugger/CMakeLists.txt e35de89 
  tools/debugger/debug-message-view.h ae745db 
  tools/debugger/debug-message-view.cpp ea09d79 
  tools/debugger/main-window.cpp 490f803 
  tools/debugger/telepathy-process.h PRE-CREATION 
  tools/debugger/telepathy-process.cpp PRE-CREATION 

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


Testing
---

Works as previously.


Thanks,

Alexandr Akulich

Re: Review Request 128847: [ktp-common-internals] [debugger] Split logic and UI

2016-09-06 Thread Alexandr Akulich 

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

(Updated Sept. 6, 2016, 6 p.m.)


Review request for Telepathy.


Changes
---

Removed m_process member from the debug view class.
Thanks to Anthony Fieroni.


Repository: ktp-common-internals


Description
---

The main goal of this change is to split logic and UI parts

This is the first step in direction to debugger, which:
1) works with any Telepathy process with DebugInterface support;
2) detects new processess "on fly";
3) has no hardcoded services;
4) shows one process just once, independently of number of dbus services, 
registered by the process.

The change also opens a way to a QML-based UI at some point in future.

Questionable thing is the "TelepathyProcess" class name.
TelepathyService does not fit, because:
1) Single process can expose a number of services (e.g. MissionControl),
2) The debug interface is applicable to any telepathy application, including 
clients, so word "Service" (which is not associated with clients) would mislead.


Diffs (updated)
-

  tools/debugger/CMakeLists.txt e35de89 
  tools/debugger/debug-message-view.h ae745db 
  tools/debugger/debug-message-view.cpp ea09d79 
  tools/debugger/main-window.cpp 490f803 
  tools/debugger/telepathy-process.h PRE-CREATION 
  tools/debugger/telepathy-process.cpp PRE-CREATION 

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


Testing
---

Works as previously.


Thanks,

Alexandr Akulich

Re: Review Request 128847: [ktp-common-internals] [debugger] Split logic and UI

2016-09-06 Thread Alexandr Akulich 

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




tools/debugger/debug-message-view.cpp (line 83)


Thank you. I leaked this too early. :)


- Alexandr Akulich


On Sept. 6, 2016, 5:26 p.m., Alexandr Akulich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128847/
> ---
> 
> (Updated Sept. 6, 2016, 5:26 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-common-internals
> 
> 
> Description
> ---
> 
> The main goal of this change is to split logic and UI parts
> 
> This is the first step in direction to debugger, which:
> 1) works with any Telepathy process with DebugInterface support;
> 2) detects new processess "on fly";
> 3) has no hardcoded services;
> 4) shows one process just once, independently of number of dbus services, 
> registered by the process.
> 
> The change also opens a way to a QML-based UI at some point in future.
> 
> Questionable thing is the "TelepathyProcess" class name.
> TelepathyService does not fit, because:
> 1) Single process can expose a number of services (e.g. MissionControl),
> 2) The debug interface is applicable to any telepathy application, including 
> clients, so word "Service" (which is not associated with clients) would 
> mislead.
> 
> 
> Diffs
> -
> 
>   tools/debugger/CMakeLists.txt e35de89 
>   tools/debugger/debug-message-view.h ae745db 
>   tools/debugger/debug-message-view.cpp ea09d79 
>   tools/debugger/main-window.cpp 490f803 
>   tools/debugger/telepathy-process.h PRE-CREATION 
>   tools/debugger/telepathy-process.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/128847/diff/
> 
> 
> Testing
> ---
> 
> Works as previously.
> 
> 
> Thanks,
> 
> Alexandr Akulich
> 
>

Re: Review Request 128847: [ktp-common-internals] [debugger] Split logic and UI

2016-09-06 Thread Anthony Fieroni 

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




tools/debugger/debug-message-view.cpp (line 83)


Why you have class variable m_process, you use it only here?


- Anthony Fieroni


On Септ. 6, 2016, 3:26 след обяд, Alexandr Akulich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128847/
> ---
> 
> (Updated Септ. 6, 2016, 3:26 след обяд)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-common-internals
> 
> 
> Description
> ---
> 
> The main goal of this change is to split logic and UI parts
> 
> This is the first step in direction to debugger, which:
> 1) works with any Telepathy process with DebugInterface support;
> 2) detects new processess "on fly";
> 3) has no hardcoded services;
> 4) shows one process just once, independently of number of dbus services, 
> registered by the process.
> 
> The change also opens a way to a QML-based UI at some point in future.
> 
> Questionable thing is the "TelepathyProcess" class name.
> TelepathyService does not fit, because:
> 1) Single process can expose a number of services (e.g. MissionControl),
> 2) The debug interface is applicable to any telepathy application, including 
> clients, so word "Service" (which is not associated with clients) would 
> mislead.
> 
> 
> Diffs
> -
> 
>   tools/debugger/CMakeLists.txt e35de89 
>   tools/debugger/debug-message-view.h ae745db 
>   tools/debugger/debug-message-view.cpp ea09d79 
>   tools/debugger/main-window.cpp 490f803 
>   tools/debugger/telepathy-process.h PRE-CREATION 
>   tools/debugger/telepathy-process.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/128847/diff/
> 
> 
> Testing
> ---
> 
> Works as previously.
> 
> 
> Thanks,
> 
> Alexandr Akulich
> 
>