--===============4046467133727548231== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit ----------------------------------------------------------- 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 > > --===============4046467133727548231== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit
This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114737/

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 January 4th, 2014, 4:54 p.m. UTC, Wolfgang Bauer wrote:

Review request for kde-workspace and David Stephen Hubner.
By Wolfgang Bauer.

Updated Jan. 4, 2014, 4:54 p.m.

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

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.

Diffs

  • kinfocenter/Modules/opengl/opengl.cpp (8901957)

View Diff

--===============4046467133727548231==--