Review Request 114737: KInfocenter/OpenGL: reimplement the ReadPipe() function with QProcess

[Wolfgang Bauer]

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

Review request for kde-workspace and David Stephen Hubner.


Repository: kde-workspace


Description
---

This patch reimplements the ReadPipe() function by using QProcess instead of 
popen().
This should make it more portable.

As a positive side-effect, this also removes those "sh: lspci: command not 
found." messages when run in Konsole and lspci is not in the user's path.

This was suggested on the kde-core-devel mailinglist in November:
http://lists.kde.org/?l=kde-core-devel&m=138407113011843&w=2
http://lists.kde.org/?l=kde-core-devel&m=138409755820003&w=2


Diffs
-

  kinfocenter/Modules/opengl/opengl.cpp 8901957 

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


Testing
---

Ran KInfocenter with lspci in /usr/bin/ (i.e. in the user's path) and /sbin/ 
(not in the user's path). The OpenGL module showed the 3D accelerator info 
correctly in both cases.
With lspci removed completely it showed "unknown" as expected.


Thanks,

Wolfgang Bauer

Re: Review Request 114737: KInfocenter/OpenGL: reimplement the ReadPipe() function with QProcess

[Rolf Eike Beer]

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



kinfocenter/Modules/opengl/opengl.cpp


This allocates a new object on the heap that is never freed. Just use 
"QProcess pipe;"


- Rolf Eike Beer


On Jan. 4, 2014, 9 a.m., Wolfgang Bauer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/114737/
> ---
> 
> (Updated Jan. 4, 2014, 9 a.m.)
> 
> 
> Review request for kde-workspace and David Stephen Hubner.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> This patch reimplements the ReadPipe() function by using QProcess instead of 
> popen().
> This should make it more portable.
> 
> As a positive side-effect, this also removes those "sh: lspci: command not 
> found." messages when run in Konsole and lspci is not in the user's path.
> 
> This was suggested on the kde-core-devel mailinglist in November:
> http://lists.kde.org/?l=kde-core-devel&m=138407113011843&w=2
> http://lists.kde.org/?l=kde-core-devel&m=138409755820003&w=2
> 
> 
> Diffs
> -
> 
>   kinfocenter/Modules/opengl/opengl.cpp 8901957 
> 
> Diff: https://git.reviewboard.kde.org/r/114737/diff/
> 
> 
> Testing
> ---
> 
> Ran KInfocenter with lspci in /usr/bin/ (i.e. in the user's path) and /sbin/ 
> (not in the user's path). The OpenGL module showed the 3D accelerator info 
> correctly in both cases.
> With lspci removed completely it showed "unknown" as expected.
> 
> 
> Thanks,
> 
> Wolfgang Bauer
> 
>

Re: Review Request 114737: KInfocenter/OpenGL: reimplement the ReadPipe() function with QProcess

[Wolfgang Bauer]

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

(Updated Jan. 4, 2014, 5:54 p.m.)


Review request for kde-workspace and David Stephen Hubner.


Changes
---

Addressed the mentioned issue.


Repository: kde-workspace


Description
---

This patch reimplements the ReadPipe() function by using QProcess instead of 
popen().
This should make it more portable.

As a positive side-effect, this also removes those "sh: lspci: command not 
found." messages when run in Konsole and lspci is not in the user's path.

This was suggested on the kde-core-devel mailinglist in November:
http://lists.kde.org/?l=kde-core-devel&m=138407113011843&w=2
http://lists.kde.org/?l=kde-core-devel&m=138409755820003&w=2


Diffs (updated)
-

  kinfocenter/Modules/opengl/opengl.cpp 8901957 

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


Testing
---

Ran KInfocenter with lspci in /usr/bin/ (i.e. in the user's path) and /sbin/ 
(not in the user's path). The OpenGL module showed the 3D accelerator info 
correctly in both cases.
With lspci removed completely it showed "unknown" as expected.


Thanks,

Wolfgang Bauer

Re: Review Request 114737: KInfocenter/OpenGL: reimplement the ReadPipe() function with QProcess

[Rolf Eike Beer]

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


Looks fine to me, although I have not tested it.

One minor nitpick (that doesn't deserve a new diff): you could make the 
FileName argument to that function "const QString &".

- Rolf Eike Beer


On Jan. 4, 2014, 4:54 p.m., Wolfgang Bauer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/114737/
> ---
> 
> (Updated Jan. 4, 2014, 4:54 p.m.)
> 
> 
> Review request for kde-workspace and David Stephen Hubner.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> This patch reimplements the ReadPipe() function by using QProcess instead of 
> popen().
> This should make it more portable.
> 
> As a positive side-effect, this also removes those "sh: lspci: command not 
> found." messages when run in Konsole and lspci is not in the user's path.
> 
> This was suggested on the kde-core-devel mailinglist in November:
> http://lists.kde.org/?l=kde-core-devel&m=138407113011843&w=2
> http://lists.kde.org/?l=kde-core-devel&m=138409755820003&w=2
> 
> 
> Diffs
> -
> 
>   kinfocenter/Modules/opengl/opengl.cpp 8901957 
> 
> Diff: https://git.reviewboard.kde.org/r/114737/diff/
> 
> 
> Testing
> ---
> 
> Ran KInfocenter with lspci in /usr/bin/ (i.e. in the user's path) and /sbin/ 
> (not in the user's path). The OpenGL module showed the 3D accelerator info 
> correctly in both cases.
> With lspci removed completely it showed "unknown" as expected.
> 
> 
> Thanks,
> 
> Wolfgang Bauer
> 
>

Re: Review Request 114737: KInfocenter/OpenGL: reimplement the ReadPipe() function with QProcess

[David Stephen Hubner]

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

Ship it!


Looks good to me

- David Stephen Hubner


On Jan. 4, 2014, 4:54 p.m., Wolfgang Bauer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/114737/
> ---
> 
> (Updated Jan. 4, 2014, 4:54 p.m.)
> 
> 
> Review request for kde-workspace and David Stephen Hubner.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> This patch reimplements the ReadPipe() function by using QProcess instead of 
> popen().
> This should make it more portable.
> 
> As a positive side-effect, this also removes those "sh: lspci: command not 
> found." messages when run in Konsole and lspci is not in the user's path.
> 
> This was suggested on the kde-core-devel mailinglist in November:
> http://lists.kde.org/?l=kde-core-devel&m=138407113011843&w=2
> http://lists.kde.org/?l=kde-core-devel&m=138409755820003&w=2
> 
> 
> Diffs
> -
> 
>   kinfocenter/Modules/opengl/opengl.cpp 8901957 
> 
> Diff: https://git.reviewboard.kde.org/r/114737/diff/
> 
> 
> Testing
> ---
> 
> Ran KInfocenter with lspci in /usr/bin/ (i.e. in the user's path) and /sbin/ 
> (not in the user's path). The OpenGL module showed the 3D accelerator info 
> correctly in both cases.
> With lspci removed completely it showed "unknown" as expected.
> 
> 
> Thanks,
> 
> Wolfgang Bauer
> 
>

Re: Review Request 114737: KInfocenter/OpenGL: reimplement the ReadPipe() function with QProcess

[Wolfgang Bauer]

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


I made another change because I noticed a "QProcess: Destroyed while process is 
still running." message in the terminal.
I now call QProcess::waitForFinished() instead of QProcess::waitForReadyRead().
This works as well, and gets rid of that warning.

- Wolfgang Bauer


On Jan. 4, 2014, 5:54 p.m., Wolfgang Bauer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/114737/
> ---
> 
> (Updated Jan. 4, 2014, 5:54 p.m.)
> 
> 
> Review request for kde-workspace and David Stephen Hubner.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> This patch reimplements the ReadPipe() function by using QProcess instead of 
> popen().
> This should make it more portable.
> 
> As a positive side-effect, this also removes those "sh: lspci: command not 
> found." messages when run in Konsole and lspci is not in the user's path.
> 
> This was suggested on the kde-core-devel mailinglist in November:
> http://lists.kde.org/?l=kde-core-devel&m=138407113011843&w=2
> http://lists.kde.org/?l=kde-core-devel&m=138409755820003&w=2
> 
> 
> Diffs
> -
> 
>   kinfocenter/Modules/opengl/opengl.cpp 8901957 
> 
> Diff: https://git.reviewboard.kde.org/r/114737/diff/
> 
> 
> Testing
> ---
> 
> Ran KInfocenter with lspci in /usr/bin/ (i.e. in the user's path) and /sbin/ 
> (not in the user's path). The OpenGL module showed the 3D accelerator info 
> correctly in both cases.
> With lspci removed completely it showed "unknown" as expected.
> 
> 
> Thanks,
> 
> Wolfgang Bauer
> 
>

Re: Review Request 114737: KInfocenter/OpenGL: reimplement the ReadPipe() function with QProcess

[Wolfgang Bauer]

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

(Updated Jan. 26, 2014, 10:43 a.m.)


Review request for kde-workspace and David Stephen Hubner.


Changes
---

- changed the type of the FileName parameter to const QString &
- call QProcess::waitForFinished() instead of QProcess::waitForReadyRead() to 
get rid of a runtime warning ("QProcess: Destroyed while process is still 
running.")


Repository: kde-workspace


Description
---

This patch reimplements the ReadPipe() function by using QProcess instead of 
popen().
This should make it more portable.

As a positive side-effect, this also removes those "sh: lspci: command not 
found." messages when run in Konsole and lspci is not in the user's path.

This was suggested on the kde-core-devel mailinglist in November:
http://lists.kde.org/?l=kde-core-devel&m=138407113011843&w=2
http://lists.kde.org/?l=kde-core-devel&m=138409755820003&w=2


Diffs (updated)
-

  kinfocenter/Modules/opengl/opengl.cpp 8901957 

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


Testing
---

Ran KInfocenter with lspci in /usr/bin/ (i.e. in the user's path) and /sbin/ 
(not in the user's path). The OpenGL module showed the 3D accelerator info 
correctly in both cases.
With lspci removed completely it showed "unknown" as expected.


Thanks,

Wolfgang Bauer

Re: Review Request 114737: KInfocenter/OpenGL: reimplement the ReadPipe() function with QProcess

[Wolfgang Bauer]

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

(Updated March 7, 2014, 11:02 p.m.)


Status
--

This change has been marked as submitted.


Review request for kde-workspace and David Stephen Hubner.


Repository: kde-workspace


Description
---

This patch reimplements the ReadPipe() function by using QProcess instead of 
popen().
This should make it more portable.

As a positive side-effect, this also removes those "sh: lspci: command not 
found." messages when run in Konsole and lspci is not in the user's path.

This was suggested on the kde-core-devel mailinglist in November:
http://lists.kde.org/?l=kde-core-devel&m=138407113011843&w=2
http://lists.kde.org/?l=kde-core-devel&m=138409755820003&w=2


Diffs
-

  kinfocenter/Modules/opengl/opengl.cpp 8901957 

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


Testing
---

Ran KInfocenter with lspci in /usr/bin/ (i.e. in the user's path) and /sbin/ 
(not in the user's path). The OpenGL module showed the 3D accelerator info 
correctly in both cases.
With lspci removed completely it showed "unknown" as expected.


Thanks,

Wolfgang Bauer

Re: Review Request 114737: KInfocenter/OpenGL: reimplement the ReadPipe() function with QProcess

[Commit Hook]

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


This review has been submitted with commit 
76d9995933f23acdef1dbf34a62649f149561e73 by Wolfgang Bauer to branch master.

- Commit Hook


On Jan. 26, 2014, 9:43 a.m., Wolfgang Bauer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/114737/
> ---
> 
> (Updated Jan. 26, 2014, 9:43 a.m.)
> 
> 
> Review request for kde-workspace and David Stephen Hubner.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> This patch reimplements the ReadPipe() function by using QProcess instead of 
> popen().
> This should make it more portable.
> 
> As a positive side-effect, this also removes those "sh: lspci: command not 
> found." messages when run in Konsole and lspci is not in the user's path.
> 
> This was suggested on the kde-core-devel mailinglist in November:
> http://lists.kde.org/?l=kde-core-devel&m=138407113011843&w=2
> http://lists.kde.org/?l=kde-core-devel&m=138409755820003&w=2
> 
> 
> Diffs
> -
> 
>   kinfocenter/Modules/opengl/opengl.cpp 8901957 
> 
> Diff: https://git.reviewboard.kde.org/r/114737/diff/
> 
> 
> Testing
> ---
> 
> Ran KInfocenter with lspci in /usr/bin/ (i.e. in the user's path) and /sbin/ 
> (not in the user's path). The OpenGL module showed the 3D accelerator info 
> correctly in both cases.
> With lspci removed completely it showed "unknown" as expected.
> 
> 
> Thanks,
> 
> Wolfgang Bauer
> 
>