Code review comment for lp:~abreu-alexandre/unity-webapps-qml/new-content-hub-api

Revision history for this message
Alberto Mardegan (mardy) wrote :

L646: + if (this._modelAdaptor) {
I guess you wanted "if (!..."

L670: Isn't it possible to avoid building the code this way, and use backendDelegate.createQmlObject() as you did for the AccountServiceModel? (I guess you had your reasons not to do it this way, please explain :-) )

L673-678 (but also later): I'd put the ";" at the end of the line, rather than at the beginning. Otherwise if the first filter is not set and the second one is, you'd end up with code like "ContentPeerModel { ; handler: ..."

L952: I guess the result of the expression must be a boolean, so maybe write it as
  this._isDefaultPeer = content && content.isDefaultPeer

I didn't test it yet, will do that soon.

review: Needs Fixing

« Back to merge proposal