[Differential] [Updated] D770: Bug 350183 - Support of $VARIABLE in environment settings

[kfunk (Kevin Funk)]

kfunk added a comment.

Looks good already, just a few issues.

Thanks


INLINE COMMENTS
  util/environmentgrouplist.cpp:210 Coding style:
  * Space after keyword, here and below
  * `{` on same line, here and below
  util/environmentgrouplist.h:26 Forward decl is enough
  util/tests/test_environment.cpp:1 Add license header
  util/tests/test_environment.cpp:7 Newline after this line
  util/tests/test_environment.cpp:8 Please use data-driven tests here.
  
  This is a perfect fit.
  
  See: http://doc.qt.io/qt-5/qttestlib-tutorial2-example.html
  util/tests/test_environment.cpp:10 I'd suggest to build your own `QProcessEnvironment` (cf. `QProcessEnvironment::insert`, etc.), and pre-fill with values for PATH/HOME.
  
  Then testing code gets easier, and you can use plain strings.
  util/tests/test_environment.h:1 Add license header

REPOSITORY
  rKDEVPLATFORM KDevPlatform

REVISION DETAIL
  https://phabricator.kde.org/D770

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: apuzio, kfunk
Cc: kdevelop-devel, arrowdodger