Splitting kde-baseapps?

2016-09-14 Thread Burkhard Lück
Hi,

what is the future of the kde-basapps repo?

It consists of:

1) Konqueror code + docbooks in subdirs doc, konq-plugins,  konqueror  + lib

2) kfind code + docbook in subdir kfind

3) kdepasswd (don't we have already a similar dialog in frameworks/
kwidgetaddons?)

4) kdialog

5) keditbookmarks (documentation is part of konqueror docbooks)

See also https://git.reviewboard.kde.org/r/128803/

-- 
Burkhard Lück



Re: Review Request 128686: Proofread + update bookmarks kcm docbook

2016-09-14 Thread Burkhard Lück

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

(Updated Sept. 14, 2016, 1:46 nachm.)


Status
--

This change has been discarded.


Review request for Documentation, KDE Base Apps, Plasma, and David Faure.


Repository: plasma-desktop


Description
---

proofread + update

code in kde-baseapps - docbook in plasma-desktop?


Diffs
-

  doc/kcontrol/bookmarks/index.docbook 173222c 

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


Testing
---

passes checkXML5


Thanks,

Burkhard Lück



Re: Review Request 128687: Proofread + update filemanager kcm docbook

2016-09-14 Thread Burkhard Lück

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

(Updated Sept. 14, 2016, 1:47 nachm.)


Status
--

This change has been discarded.


Review request for Documentation, KDE Base Apps, Plasma, and David Faure.


Repository: plasma-desktop


Description
---

proofread + update

code in kde-baseapps - docbook in plasma-desktop?


Diffs
-

  doc/kcontrol/filemanager/index.docbook 2ac80b6 

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


Testing
---

passes checkXML5


Thanks,

Burkhard Lück



Re: Review Request 128688: Proofread + update khtml-behavior kcm docbook

2016-09-14 Thread Burkhard Lück

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

(Updated Sept. 14, 2016, 1:48 nachm.)


Status
--

This change has been discarded.


Review request for Documentation, KDE Base Apps, Plasma, and David Faure.


Repository: plasma-desktop


Description
---

proofread + update
remove unused entity

code in kde-baseapps - docbook in plasma-desktop?


Diffs
-

  doc/kcontrol/khtml-behavior/index.docbook ca39472 

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


Testing
---

passes checkXML5


Thanks,

Burkhard Lück



Re: Review Request 128689: Proofread + update kcmcss kcm docbook

2016-09-14 Thread Burkhard Lück

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

(Updated Sept. 14, 2016, 1:48 nachm.)


Status
--

This change has been discarded.


Review request for Documentation, KDE Base Apps, Plasma, and David Faure.


Repository: plasma-desktop


Description
---

proofread + update

code in kde-baseapps - docbook in plasma-desktop?


Diffs
-

  doc/kcontrol/kcmcss/index.docbook c63e83c 

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


Testing
---

passes checkXML5


Thanks,

Burkhard Lück



Re: Review Request 128691: Proofread + update history kcm docbook

2016-09-14 Thread Burkhard Lück

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

(Updated Sept. 14, 2016, 1:49 nachm.)


Status
--

This change has been discarded.


Review request for Documentation, KDE Base Apps, Plasma, and David Faure.


Repository: plasma-desktop


Description
---

proofread + update

code in kde-baseapps - docbook in plasma-desktop?


Diffs
-

  doc/kcontrol/history/index.docbook 27f805c 

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


Testing
---

passes checkXML5


Thanks,

Burkhard Lück



Re: Review Request 128693: Proofread + update khtml-java-js kcm docbook

2016-09-14 Thread Burkhard Lück

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

(Updated Sept. 14, 2016, 1:49 nachm.)


Status
--

This change has been discarded.


Review request for Documentation, KDE Base Apps, Plasma, and David Faure.


Repository: plasma-desktop


Description
---

proofread + update
remove unused entity nsplugins-kcontrol
add debug info
use entity javascript

code in kde-baseapps - docbook in plasma-desktop?


Diffs
-

  doc/kcontrol/khtml-java-js/index.docbook d707fcb 

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


Testing
---

passes checkXML5


Thanks,

Burkhard Lück



Re: Review Request 128690: Proofread + update khtml-adblock kcm docbook

2016-09-14 Thread Burkhard Lück

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

(Updated Sept. 14, 2016, 1:48 nachm.)


Status
--

This change has been discarded.


Review request for Documentation, KDE Base Apps, Plasma, and David Faure.


Repository: plasma-desktop


Description
---

proofread + update
remove unused entity

code in kde-baseapps - docbook in plasma-desktop?


Diffs
-

  doc/kcontrol/khtml-adblock/index.docbook 1868626 

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


Testing
---

passes checkXML5


Thanks,

Burkhard Lück



Re: Splitting kde-baseapps?

2016-09-14 Thread Luigi Toscano
On Wednesday, 14 September 2016 15:46:28 CEST Burkhard Lück wrote:
> Hi,
> 
> what is the future of the kde-basapps repo?
> 
> It consists of:
> 
> 1) Konqueror code + docbooks in subdirs doc, konq-plugins,  konqueror  + lib
> 
> 2) kfind code + docbook in subdir kfind
> 
> 3) kdepasswd (don't we have already a similar dialog in frameworks/
> kwidgetaddons?)
> 
> 4) kdialog
> 
> 5) keditbookmarks (documentation is part of konqueror docbooks)
> 
> See also https://git.reviewboard.kde.org/r/128803/


I spoke briefly with David about this. The simpler scripts that I tried to 
split the repo simply forget something (like the history of doc/ which was 
moved around; I tried with kfind, which is already prepared for the split from 
the structural point of view). I'm waiting for a script used by the PIM team 
to see if it improves that.

-- 
Luigi



Re: Review Request 128684: Proofread + update khtml-general kcm docbook

2016-09-14 Thread Burkhard Lück


> On Aug. 16, 2016, 7:05 vorm., David Faure wrote:
> > doc/kcontrol/khtml-general/index.docbook, line 51
> > 
> >
> > Well, qt5-webkit and kwebkitpart do still exist. They're just not 
> > really maintained (but then again that is a problem for konqueror itself as 
> > well, especially due to being built on top of deprecated web engines...). 
> > (I hate this situation.)
> 
> Burkhard Lück wrote:
> Thanks to your hints I just detected that kwebkitpart has a frameworks 
> branch. Building this branch I get the webkit engine back in konqueror kf5.
> But kwebkitpart frameworks branch is unreleased, stable/released branch 
> is 1.3 kde4 based, master as well.
> So should I update the docbooks for konqueror with kwebkitpart/frameworks 
> branch?
> 
> Burkhard Lück wrote:
> > So should I update the docbooks for konqueror with 
> kwebkitpart/frameworks branch?
> 
> kde-doc-english: please comment/answer my question
> thanks
> 
> Yuri Chornoivan wrote:
> +1
> 
> Luigi Toscano wrote:
> The kwerbkitpart is under construction, and there may be a qwebengine 
> part instead or in addition. I would leave that part commented with a 
> reminder to revisit it before the documentation freeze.

Let's do it the other way round, leave kwebkitpart for now and revisit before 
documentation freeze. 
That we we get the konqueror except the kwebkit related stuff immediately up to 
date and have minimal changes for now.


- Burkhard


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


On Aug. 15, 2016, 11:40 vorm., Burkhard Lück wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128684/
> ---
> 
> (Updated Aug. 15, 2016, 11:40 vorm.)
> 
> 
> Review request for Documentation, KDE Base Apps, Plasma, and David Faure.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> proofread + update
> comment webkit
> 
> code in kde-baseapps - docbook in plasma-desktop?
> 
> 
> Diffs
> -
> 
>   doc/kcontrol/khtml-general/index.docbook 1b9c80e 
> 
> Diff: https://git.reviewboard.kde.org/r/128684/diff/
> 
> 
> Testing
> ---
> 
> passes checkXML5
> 
> 
> Thanks,
> 
> Burkhard Lück
> 
>



Re: Review Request 128685: Proofread + update performance kcm docbook

2016-09-14 Thread Burkhard Lück

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

(Updated Sept. 14, 2016, 2:25 nachm.)


Status
--

This change has been discarded.


Review request for Documentation, KDE Base Apps, Plasma, and David Faure.


Repository: plasma-desktop


Description
---

proofread + update
remove obsolete comment

code in kde-baseapps - docbook in plasma-desktop?


Diffs
-

  doc/kcontrol/performance/index.docbook 5ac5055 

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


Testing
---

passes checkXML5


Thanks,

Burkhard Lück



Review Request 128909: initial, minimal support for OS X

2016-09-14 Thread René J . V . Bertin

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

Review request for KDE Base Apps and KDE Software on Mac OS X.


Repository: libksysguard


Description
---

This patch implements initial and rather minimal support for OS X, for now 
focussing purely on process information. That feature is justified as it is 
used by KDevelop in order to obtain the list of processes one can attach a 
debugger to.

Mac OS X is tricky because it requires special privileges in order to obtain 
certain types of information for any running process. For example, even 
obtaining the number of threads spawned by a foreign process requires 
privileges that aren't trivial to set up. I've prepared the terrain, but also 
implemented a fallback strategy that calls `ps` to be sure that crucial 
information like the command name is available for all processes.


Diffs
-

  processcore/CMakeLists.txt e7c9263 
  processcore/Info.plist PRE-CREATION 
  processcore/helper.cpp d54c8e1 
  processcore/processes_darwin_p.cpp PRE-CREATION 
  processcore/processes_local_p.cpp 2bc123f 

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


Testing
---

On OS X 10.9.5 with Frameworks 5.24.0 and Qt 5.6.1


Thanks,

René J.V. Bertin



Re: Review Request 128684: Proofread + update khtml-general kcm docbook

2016-09-14 Thread Burkhard Lück


> On Aug. 16, 2016, 7:05 vorm., David Faure wrote:
> > doc/kcontrol/khtml-general/index.docbook, line 51
> > 
> >
> > Well, qt5-webkit and kwebkitpart do still exist. They're just not 
> > really maintained (but then again that is a problem for konqueror itself as 
> > well, especially due to being built on top of deprecated web engines...). 
> > (I hate this situation.)
> 
> Burkhard Lück wrote:
> Thanks to your hints I just detected that kwebkitpart has a frameworks 
> branch. Building this branch I get the webkit engine back in konqueror kf5.
> But kwebkitpart frameworks branch is unreleased, stable/released branch 
> is 1.3 kde4 based, master as well.
> So should I update the docbooks for konqueror with kwebkitpart/frameworks 
> branch?
> 
> Burkhard Lück wrote:
> > So should I update the docbooks for konqueror with 
> kwebkitpart/frameworks branch?
> 
> kde-doc-english: please comment/answer my question
> thanks
> 
> Yuri Chornoivan wrote:
> +1
> 
> Luigi Toscano wrote:
> The kwerbkitpart is under construction, and there may be a qwebengine 
> part instead or in addition. I would leave that part commented with a 
> reminder to revisit it before the documentation freeze.
> 
> Burkhard Lück wrote:
> Let's do it the other way round, leave kwebkitpart for now and revisit 
> before documentation freeze. 
> That we we get the konqueror except the kwebkit related stuff immediately 
> up to date and have minimal changes for now.

in https://bugs.kde.org/show_bug.cgi?id=368705#c1 Christoph says:
 
 "Konqueror and the WebKit part will be released as Qt5 versions starting with 
KDE Applications 16.12 release."
 
 so we should stick with the kwebkitpart for now


- Burkhard


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


On Aug. 15, 2016, 11:40 vorm., Burkhard Lück wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128684/
> ---
> 
> (Updated Aug. 15, 2016, 11:40 vorm.)
> 
> 
> Review request for Documentation, KDE Base Apps, Plasma, and David Faure.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> proofread + update
> comment webkit
> 
> code in kde-baseapps - docbook in plasma-desktop?
> 
> 
> Diffs
> -
> 
>   doc/kcontrol/khtml-general/index.docbook 1b9c80e 
> 
> Diff: https://git.reviewboard.kde.org/r/128684/diff/
> 
> 
> Testing
> ---
> 
> passes checkXML5
> 
> 
> Thanks,
> 
> Burkhard Lück
> 
>



Re: Review Request 128684: Proofread + update khtml-general kcm docbook

2016-09-14 Thread Christoph Feck


> On Aug. 16, 2016, 9:05 a.m., David Faure wrote:
> > doc/kcontrol/khtml-general/index.docbook, line 51
> > 
> >
> > Well, qt5-webkit and kwebkitpart do still exist. They're just not 
> > really maintained (but then again that is a problem for konqueror itself as 
> > well, especially due to being built on top of deprecated web engines...). 
> > (I hate this situation.)
> 
> Burkhard Lück wrote:
> Thanks to your hints I just detected that kwebkitpart has a frameworks 
> branch. Building this branch I get the webkit engine back in konqueror kf5.
> But kwebkitpart frameworks branch is unreleased, stable/released branch 
> is 1.3 kde4 based, master as well.
> So should I update the docbooks for konqueror with kwebkitpart/frameworks 
> branch?
> 
> Burkhard Lück wrote:
> > So should I update the docbooks for konqueror with 
> kwebkitpart/frameworks branch?
> 
> kde-doc-english: please comment/answer my question
> thanks
> 
> Yuri Chornoivan wrote:
> +1
> 
> Luigi Toscano wrote:
> The kwerbkitpart is under construction, and there may be a qwebengine 
> part instead or in addition. I would leave that part commented with a 
> reminder to revisit it before the documentation freeze.
> 
> Burkhard Lück wrote:
> Let's do it the other way round, leave kwebkitpart for now and revisit 
> before documentation freeze. 
> That we we get the konqueror except the kwebkit related stuff immediately 
> up to date and have minimal changes for now.
> 
> Burkhard Lück wrote:
> in https://bugs.kde.org/show_bug.cgi?id=368705#c1 Christoph says:
>  
>  "Konqueror and the WebKit part will be released as Qt5 versions starting 
> with KDE Applications 16.12 release."
>  
>  so we should stick with the kwebkitpart for now

Sorry, I was too fast. kwebkitpart is in extragear, so it will not be 
automatically released.


- Christoph


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


On Aug. 15, 2016, 1:40 p.m., Burkhard Lück wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128684/
> ---
> 
> (Updated Aug. 15, 2016, 1:40 p.m.)
> 
> 
> Review request for Documentation, KDE Base Apps, Plasma, and David Faure.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> proofread + update
> comment webkit
> 
> code in kde-baseapps - docbook in plasma-desktop?
> 
> 
> Diffs
> -
> 
>   doc/kcontrol/khtml-general/index.docbook 1b9c80e 
> 
> Diff: https://git.reviewboard.kde.org/r/128684/diff/
> 
> 
> Testing
> ---
> 
> passes checkXML5
> 
> 
> Thanks,
> 
> Burkhard Lück
> 
>



Re: Review Request 128909: initial, minimal support for OS X

2016-09-14 Thread Anthony Fieroni

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




processcore/processes_darwin_p.cpp (line 104)


*inline* must be used in function definition not in declaration.


- Anthony Fieroni


On Септ. 14, 2016, 6:28 след обяд, René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128909/
> ---
> 
> (Updated Септ. 14, 2016, 6:28 след обяд)
> 
> 
> Review request for KDE Base Apps and KDE Software on Mac OS X.
> 
> 
> Repository: libksysguard
> 
> 
> Description
> ---
> 
> This patch implements initial and rather minimal support for OS X, for now 
> focussing purely on process information. That feature is justified as it is 
> used by KDevelop in order to obtain the list of processes one can attach a 
> debugger to.
> 
> Mac OS X is tricky because it requires special privileges in order to obtain 
> certain types of information for any running process. For example, even 
> obtaining the number of threads spawned by a foreign process requires 
> privileges that aren't trivial to set up. I've prepared the terrain, but also 
> implemented a fallback strategy that calls `ps` to be sure that crucial 
> information like the command name is available for all processes.
> 
> 
> Diffs
> -
> 
>   processcore/CMakeLists.txt e7c9263 
>   processcore/Info.plist PRE-CREATION 
>   processcore/helper.cpp d54c8e1 
>   processcore/processes_darwin_p.cpp PRE-CREATION 
>   processcore/processes_local_p.cpp 2bc123f 
> 
> Diff: https://git.reviewboard.kde.org/r/128909/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9.5 with Frameworks 5.24.0 and Qt 5.6.1
> 
> 
> File Attachments
> 
> 
> Attach to Process widget in KDevelop
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/09/14/47468811-58cb-40b8-a735-4dd86dce98e1__Screen_Shot_2016-09-14_at_17.38.22.png
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>



Re: Review Request 128909: initial, minimal support for OS X

2016-09-14 Thread René J . V . Bertin


> On Sept. 14, 2016, 7:33 p.m., Anthony Fieroni wrote:
> > processcore/processes_darwin_p.cpp, line 104
> > 
> >
> > *inline* must be used in function definition not in declaration.

I didn't see the point of the way the keyword is being used here either, but I 
aligned myself to the current practice.

So I take it I shouldn't only correct `argMax()`?


- René J.V.


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


On Sept. 14, 2016, 5:28 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128909/
> ---
> 
> (Updated Sept. 14, 2016, 5:28 p.m.)
> 
> 
> Review request for KDE Base Apps and KDE Software on Mac OS X.
> 
> 
> Repository: libksysguard
> 
> 
> Description
> ---
> 
> This patch implements initial and rather minimal support for OS X, for now 
> focussing purely on process information. That feature is justified as it is 
> used by KDevelop in order to obtain the list of processes one can attach a 
> debugger to.
> 
> Mac OS X is tricky because it requires special privileges in order to obtain 
> certain types of information for any running process. For example, even 
> obtaining the number of threads spawned by a foreign process requires 
> privileges that aren't trivial to set up. I've prepared the terrain, but also 
> implemented a fallback strategy that calls `ps` to be sure that crucial 
> information like the command name is available for all processes.
> 
> 
> Diffs
> -
> 
>   processcore/CMakeLists.txt e7c9263 
>   processcore/Info.plist PRE-CREATION 
>   processcore/helper.cpp d54c8e1 
>   processcore/processes_darwin_p.cpp PRE-CREATION 
>   processcore/processes_local_p.cpp 2bc123f 
> 
> Diff: https://git.reviewboard.kde.org/r/128909/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9.5 with Frameworks 5.24.0 and Qt 5.6.1
> 
> 
> File Attachments
> 
> 
> Attach to Process widget in KDevelop
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/09/14/47468811-58cb-40b8-a735-4dd86dce98e1__Screen_Shot_2016-09-14_at_17.38.22.png
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>



Review Request 128910: [kio_trash] Fill in UDS_LOCAL_PATH in UDSEntry

2016-09-14 Thread Wolfgang Bauer

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

Review request for KDE Runtime, KDE Frameworks and David Faure.


Bugs: 208625, 272249, 329155, and 368104
https://bugs.kde.org/show_bug.cgi?id=208625
https://bugs.kde.org/show_bug.cgi?id=272249
https://bugs.kde.org/show_bug.cgi?id=329155
https://bugs.kde.org/show_bug.cgi?id=368104


Repository: kio


Description
---

Not doing so results in PreviewJob assuming the files/folders are remote and 
copying them to /tmp when generating previews.
This is especially annoying and dangerous if there are large folders in the 
trash.

KDE4's kio_trash in kde-runtime has the same problem, as the code is basically 
the same, the same patch fixes it too.


Diffs
-

  src/ioslaves/trash/kio_trash.cpp 3810941 

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


Testing
---

Trash some files/folders, open dolphin, navigate to trash:/, and hover over 
them.
Previews are still generated (if enabled), and the files/folders are not copied 
to /tmp any more.

Also tried with files on an USB stick, where a folder .Trash-XXX is used as 
trash.


Thanks,

Wolfgang Bauer



Re: Review Request 128909: initial, minimal support for OS X

2016-09-14 Thread René J . V . Bertin

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

(Updated Sept. 14, 2016, 9:37 p.m.)


Review request for KDE Base Apps and KDE Software on Mac OS X.


Changes
---

inline issue hopefully fixed


Repository: libksysguard


Description
---

This patch implements initial and rather minimal support for OS X, for now 
focussing purely on process information. That feature is justified as it is 
used by KDevelop in order to obtain the list of processes one can attach a 
debugger to.

Mac OS X is tricky because it requires special privileges in order to obtain 
certain types of information for any running process. For example, even 
obtaining the number of threads spawned by a foreign process requires 
privileges that aren't trivial to set up. I've prepared the terrain, but also 
implemented a fallback strategy that calls `ps` to be sure that crucial 
information like the command name is available for all processes.


Diffs (updated)
-

  processcore/CMakeLists.txt e7c9263 
  processcore/Info.plist PRE-CREATION 
  processcore/helper.cpp d54c8e1 
  processcore/processes_darwin_p.cpp PRE-CREATION 
  processcore/processes_local_p.cpp 2bc123f 

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


Testing
---

On OS X 10.9.5 with Frameworks 5.24.0 and Qt 5.6.1


File Attachments


Attach to Process widget in KDevelop
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/09/14/47468811-58cb-40b8-a735-4dd86dce98e1__Screen_Shot_2016-09-14_at_17.38.22.png


Thanks,

René J.V. Bertin



Re: Review Request 128909: initial, minimal support for OS X

2016-09-14 Thread Aleix Pol Gonzalez

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



Other than that, the patch LGTM.


processcore/helper.cpp (line 133)


`KAUTH_HELPER_MAIN` doesn't work on OS X?


- Aleix Pol Gonzalez


On Sept. 14, 2016, 9:37 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128909/
> ---
> 
> (Updated Sept. 14, 2016, 9:37 p.m.)
> 
> 
> Review request for KDE Base Apps and KDE Software on Mac OS X.
> 
> 
> Repository: libksysguard
> 
> 
> Description
> ---
> 
> This patch implements initial and rather minimal support for OS X, for now 
> focussing purely on process information. That feature is justified as it is 
> used by KDevelop in order to obtain the list of processes one can attach a 
> debugger to.
> 
> Mac OS X is tricky because it requires special privileges in order to obtain 
> certain types of information for any running process. For example, even 
> obtaining the number of threads spawned by a foreign process requires 
> privileges that aren't trivial to set up. I've prepared the terrain, but also 
> implemented a fallback strategy that calls `ps` to be sure that crucial 
> information like the command name is available for all processes.
> 
> 
> Diffs
> -
> 
>   processcore/CMakeLists.txt e7c9263 
>   processcore/Info.plist PRE-CREATION 
>   processcore/helper.cpp d54c8e1 
>   processcore/processes_darwin_p.cpp PRE-CREATION 
>   processcore/processes_local_p.cpp 2bc123f 
> 
> Diff: https://git.reviewboard.kde.org/r/128909/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9.5 with Frameworks 5.24.0 and Qt 5.6.1
> 
> 
> File Attachments
> 
> 
> Attach to Process widget in KDevelop
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/09/14/47468811-58cb-40b8-a735-4dd86dce98e1__Screen_Shot_2016-09-14_at_17.38.22.png
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>



Re: Review Request 128909: initial, minimal support for OS X

2016-09-14 Thread Thiago Macieira
On quarta-feira, 14 de setembro de 2016 17:58:26 PDT René J.V. Bertin wrote:
> > On Sept. 14, 2016, 7:33 p.m., Anthony Fieroni wrote:
> > > processcore/processes_darwin_p.cpp, line 104
> > >  > > line104>> > 
> > > *inline* must be used in function definition not in declaration.
> 
> I didn't see the point of the way the keyword is being used here either, but
> I aligned myself to the current practice.
> 
> So I take it I shouldn't only correct `argMax()`?

It's all in the same .cpp, it doesn't matter. Inline is correct where it is.

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
   Software Architect - Intel Open Source Technology Center