thaliproject / postcardapp

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

Story 001 dadougla #68

Closed deadlyfingers closed 8 years ago

deadlyfingers commented 8 years ago

Identity Exchange using TRM

Review on Reviewable

mpodwysocki commented 8 years ago

cleanup.bat, line 3 [r1] (raw file): There seems to be a weird space value there...


Comments from the review on Reviewable.io

mpodwysocki commented 8 years ago

readme.md, line 18 [r1] (raw file): Is this still true? For example, could we point them to use the VC++ Build Tools Preview? https://www.microsoft.com/en-us/download/details.aspx?id=49512


Comments from the review on Reviewable.io

mpodwysocki commented 8 years ago

www/jxcore/routes/manager.js, line 8 [r1] (raw file): you should wait for the started before sending a 200


Comments from the review on Reviewable.io

mpodwysocki commented 8 years ago

www/jxcore/routes/manager.js, line 14 [r1] (raw file): Likewise, you might want to wait for a full stop before sending a 200


Comments from the review on Reviewable.io

mpodwysocki commented 8 years ago

readme.md, line 15 [r2] (raw file): You should also note that it comes directly with Node.js so no need to download node-gyp itself


Comments from the review on Reviewable.io

mpodwysocki commented 8 years ago

www/jxcore/public/scripts/app.js, line 43 [r2] (raw file): Why would socket.io be unavailable?


Comments from the review on Reviewable.io

mpodwysocki commented 8 years ago

www/jxcore/public/scripts/app.js, line 124 [r2] (raw file): Much easier to say functionToCheck && Object.prototype.toString.call(functionToCheck) === '[object Function]';


Comments from the review on Reviewable.io

deadlyfingers commented 8 years ago

Review status: 0 of 79 files reviewed at latest revision, 2 unresolved discussions.


cleanup.bat, line 3 [r1] (raw file): Arg a gremlin! I think this entire file can be ditched now though.


readme.md, line 18 [r1] (raw file): Ok I would need to test that. We should prob create an issue for doing a Windows setup readme.


Comments from the review on Reviewable.io