D20626: Refactor and cleanup
shubham updated this revision to Diff 58351. shubham edited the summary of this revision. shubham edited the test plan for this revision. shubham added a comment. Rely upon QSysInfo to retrieve the system details REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D20626?vs=56417=58351 BRANCH arcpatch-D20626 REVISION DETAIL https://phabricator.kde.org/D20626 AFFECTED FILES src/kcms/kio/useragentinfo.cpp To: shubham, dfaure Cc: pino, kde-frameworks-devel, michaelh, ngraham, bruns
D20626: Refactor and cleanup
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Please make sure to edit the commit log -- the phabricator description still says "Refactor and cleanup" ... REPOSITORY R241 KIO BRANCH cleanup REVISION DETAIL https://phabricator.kde.org/D20626 To: shubham, dfaure Cc: pino, kde-frameworks-devel, michaelh, ngraham, bruns
D20626: Refactor and cleanup
shubham edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D20626 To: shubham, dfaure Cc: pino, kde-frameworks-devel, michaelh, ngraham, bruns
D20626: Refactor and cleanup
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. - 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. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D20626 To: shubham, dfaure Cc: pino, kde-frameworks-devel, michaelh, ngraham, bruns
D20626: Refactor and cleanup
shubham added a comment. @dfaure Ping? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D20626 To: shubham, dfaure Cc: pino, kde-frameworks-devel, michaelh, ngraham, bruns
D20626: Refactor and cleanup
shubham edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D20626 To: shubham, dfaure Cc: pino, kde-frameworks-devel, michaelh, ngraham, bruns
D20626: Refactor and cleanup
pino added a comment. 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. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D20626 To: shubham, dfaure Cc: pino, kde-frameworks-devel, michaelh, ngraham, bruns
D20626: Refactor and cleanup
shubham edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D20626 To: shubham, dfaure Cc: kde-frameworks-devel, michaelh, ngraham, bruns
D20626: Refactor and cleanup
shubham edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D20626 To: shubham, dfaure Cc: kde-frameworks-devel, michaelh, ngraham, bruns
D20626: Refactor and cleanup
dfaure added a comment. 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? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D20626 To: shubham, dfaure Cc: kde-frameworks-devel, michaelh, ngraham, bruns
D20626: Refactor and cleanup
shubham created this revision. shubham added a reviewer: dfaure. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. shubham requested review of this revision. REPOSITORY R241 KIO BRANCH cleanup REVISION DETAIL https://phabricator.kde.org/D20626 AFFECTED FILES src/kcms/kio/useragentinfo.cpp To: shubham, dfaure Cc: kde-frameworks-devel, michaelh, ngraham, bruns
