D11034: CoreApplicationSettingsTest

[Johnny Jazeix]

jjazeix added a comment.


  Can you check that if you instantiate a ApplicationSettingsMock object, then use ApplicationSettings::getInstance(), it returns the mock instead of creating a new ApplicationSettings object?
  As we use it this way on other classes (like ApplicationInfo), it would be better that, for tests, the getInstance() returns the mock instead of the real one (else we'll have the same issue as we had here, writing on the real configuration).
  
  Except this point (and the small one regarding parameters ordering), it seems good to me, I'll take a closer look this week-end to integrate it

INLINE COMMENTS

> ApplicationSettings.h:13


let's keep the same format as Qt and set the parent object as last input parameter of the constructor

> ApplicationSettings.h:257
>  	/// @cond INTERNAL_DOCS
> -    explicit ApplicationSettings(QObject *parent = 0);
> +    explicit ApplicationSettings(QObject *parent = 0, const QString path =
> +         QStandardPaths::writableLocation(QStandardPaths::GenericConfigLocation) +

let's keep the same format as Qt and set the parent object as last input parameter of the constructor

REPOSITORY
  R2 GCompris

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

To: himanshuvishwakarma, jjazeix, dmadaan, rudranilbasu, timotheegiet, #gcompris
Cc: #kde_edu, #gcompris, harrymecwan, ganeshredcobra, nityanandkumar, echarruau, rahulyadav, narvaez, scagarwal, apol, timotheegiet, hkaelberer, jjazeix, bcoudoin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20180323/651269b1/attachment.html>