yellow team mailing list archive

[Gary Poster <gary.poster@xxxxxxxxxxxxx>]

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.