thaliproject / postcardapp

A sample app to demonstrate how to build Thali applications
MIT License
22 stars 5 forks source link

Story 0 using thali module 2.0.1 to fix #60 #61

Closed deadlyfingers closed 8 years ago

deadlyfingers commented 8 years ago

Thali version updated to ^2.0.1 in package.json which automatically installs the JXCore Cordova plugin.

Review on Reviewable

vjrantal commented 8 years ago

Reviewed 5 of 5 files at r1. Review status: all files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.


readme.md, line 154 [r1] (raw file): I guess in theory, this whole section in the readme isn't needed, because the jxcore cordova plugin should be automatically installed?

If this is left in, the danger is that the instructions gets outdated, for example, if we one day require newer version than 0.0.7.


www/jxcore/app.js, line 68 [r1] (raw file): If wanted, this could be implemented in a backwards-compatible way by checking how many arguments start expects and passing accordingly. Something like:

if (manager.start.length === 2) { // Don't pass the first port argument } else { // Pass the port as first argument }


www/jxcore/public/styles/main.css, line 105 [r1] (raw file): Was this change on purpose a part of this commit?


Comments from the review on Reviewable.io

deadlyfingers commented 8 years ago

Review status: all files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.


www/jxcore/app.js, line 68 [r1] (raw file): On the backward compatibility part - the start methods both seem to have 3 params...

Old function: ThaliReplicationManager.prototype.start = function (deviceName, port, dbName) New function: ThaliReplicationManager.prototype.start = function (port, dbName, deviceName)

However if we are requesting the newer version of thali then we should be safe not to need this backward compatible check anyway.


www/jxcore/public/styles/main.css, line 105 [r1] (raw file): Yes, there is an extra shadow that looks out of place on the listview with newer version of components.


Comments from the review on Reviewable.io

vjrantal commented 8 years ago

Reviewed 1 of 1 files at r2. Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from the review on Reviewable.io