Differential D20626

Rely upon QSysInfo to retrieve the system details
ClosedPublic
Actions

Authored by shubham on Apr 17 2019, 8:20 AM.

Details

Reviewers
dfaure
Commits
R241:0d2681df45f8: Rely upon QSysInfo to retrieve the system details
Summary

QSysInfo works on both unix like and windows OS's.
Better to use it to gather information about the system.

Test Plan

Ran ctest, tests executed successfully

kcmshell5 --useragent

Diff Detail

Repository
R241 KIO
Branch
sysinfo
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 11991
Build 12009: arc lint + arc unit
shubham created this revision.Apr 17 2019, 8:20 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 17 2019, 8:20 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
shubham requested review of this revision.Apr 17 2019, 8:20 AM
dfaure added a comment.Apr 17 2019, 12:32 PM

I like the idea.

The commit template has a "Test Plan" field, but it seems it was left empty - can you detail what tests you did?

shubham edited the test plan for this revision. (Show Details)Apr 19 2019, 8:16 AM
shubham edited the test plan for this revision. (Show Details)Apr 19 2019, 8:25 AM
pino added a subscriber: pino.Apr 19 2019, 8:28 AM

Also, please explicitly mention what are the changes done. "refactor and cleanup" is very vague, while saying that, for example, QSysInfo is used on all the OSes is better.

shubham edited the summary of this revision. (Show Details)Apr 24 2019, 3:50 PM

@dfaure Ping?

dfaure requested changes to this revision.Apr 25 2019, 9:17 AM
  • The description still says "Refactor and cleanup" (note that phabricator doesn't auto-update from the commit log, unless you use arc diff --verbatim, so one often has to copy/paste)
  • Running the unittests is always good, but they don't cover this code. Please actually use the KCM to test this commit. I would put some nonsense into appSysName first, to make sure I'm testing correctly, then putting back the right value, to make sure it's correct.
This revision now requires changes to proceed.Apr 25 2019, 9:17 AM
shubham edited the test plan for this revision. (Show Details)May 19 2019, 5:02 PM
dfaure accepted this revision.May 20 2019, 7:19 AM

Please make sure to edit the commit log -- the phabricator description still says "Refactor and cleanup" ...

This revision is now accepted and ready to land.May 20 2019, 7:19 AM
shubham updated this revision to Diff 58351.May 20 2019, 12:37 PM
shubham edited the summary of this revision. (Show Details)
shubham edited the test plan for this revision. (Show Details)

Rely upon QSysInfo to retrieve the system details

shubham retitled this revision from Refactor and cleanup to Rely upon QSysInfo to retrieve the system details.May 20 2019, 12:38 PM
shubham edited the summary of this revision. (Show Details)
shubham edited the test plan for this revision. (Show Details)
shubham updated this revision to Diff 58354.May 20 2019, 12:41 PM
shubham edited the summary of this revision. (Show Details)

proper rebase

dfaure added a comment.May 20 2019, 2:53 PM

Thanks. You can land the commit.

shubham added a comment.May 20 2019, 2:53 PM
This comment was removed by shubham.
dfaure closed this revision.May 21 2019, 7:25 AM

Revision Contents
Changeset List

PathPackages
Msrc/kcms/kio/useragentinfo.cpp (20 lines)

Diff 58369

View Options

src/kcms/kio/useragentinfo.cpp

Loading...