Land with changes. Thank you for accomplishing this difficult task.
In addition to the comments below, please add a request in process.rst that reviewers run both prod tests and debug tests. Then, in the Makefile, either make the default test the prod test, or make it thumb its nose at us and tell us to run one of the other targets. Gary https://codereview.appspot.com/6947057/diff/1/Makefile File Makefile (right): https://codereview.appspot.com/6947057/diff/1/Makefile#newcode293 Makefile:293: cp -r --parents */assets "$(PWD)/build-$(1)/juju-ui/assets/") Looks nicer to me, thanks. https://codereview.appspot.com/6947057/diff/1/bin/merge-files File bin/merge-files (right): https://codereview.appspot.com/6947057/diff/1/bin/merge-files#newcode50 bin/merge-files:50: paths.push(syspath.join(process.cwd(), 'app/models/charm.js')); As we discussed, please either figure out why readdir is not finding charm.js, or put an XXX and a bug, and we will come back to this. https://codereview.appspot.com/6947057/diff/1/test/index.html File test/index.html (right): https://codereview.appspot.com/6947057/diff/1/test/index.html#newcode9 test/index.html:9: <script src="/juju-ui/assets/node-event-simulate.js"></script> You explained the horror behind these two inclusions. Please add a comment so others can enjoy. https://codereview.appspot.com/6947057/ -- https://code.launchpad.net/~benji/juju-gui/bug-1088507/+merge/140020 Your team Juju GUI Hackers is requested to review the proposed merge of lp:~benji/juju-gui/bug-1088507 into lp:juju-gui. -- Mailing list: https://launchpad.net/~yellow Post to : [email protected] Unsubscribe : https://launchpad.net/~yellow More help : https://help.launchpad.net/ListHelp
