Thanks Gary for your review!
https://codereview.appspot.com/6844061/diff/1/.bzrignore File .bzrignore (right): https://codereview.appspot.com/6844061/diff/1/.bzrignore#newcode16 .bzrignore:16: build On 2012/11/19 19:00:33, gary.poster wrote: > Nice simplification and clean-up. Ohhh yeah. :O) https://codereview.appspot.com/6844061/diff/1/Makefile File Makefile (right): https://codereview.appspot.com/6844061/diff/1/Makefile#newcode100 Makefile:100: On 2012/11/19 19:00:33, gary.poster wrote: > On 2012/11/19 18:18:58, bac wrote: > > I'm concerned about separate build and install targets and having template.js > in > > a separate rule from the others. Seems complicated and twisty. > Having "build" depend on "install" also seems odd, conceptually. > Does the "install" target bring any value? That is, is there any use case for > running it alone? If not, I suggest combining it and "build" into the "build" > target (and deleting the "install" target). What do you two think of that? Ah... I see! I will delete the "install" target and change the "build" target from... build: install ... to... build: appcache $(NODE_TARGETS) build/juju-ui/templates.js yuidoc spritegen combinejs Is that ok? https://codereview.appspot.com/6844061/diff/1/lib/server.js File lib/server.js (right): https://codereview.appspot.com/6844061/diff/1/lib/server.js#newcode57 lib/server.js:57: res.sendfile('build/juju-ui/assets/' + fileName); On 2012/11/19 19:00:33, gary.poster wrote: > Interesting. I wanted the development server code to be even simpler than this, > but I can see why what you did is compelling. I would be tempted to experiment > with converting the js files to symlinks in a later branch and simplifying this > code. This is an improvement anyway, though, and maybe my hunch won't work out. I needed to change the "test-server.js" file too. The tests were broken due to the new notification stuff. I've tried to use the symbolic link approach in order to avoid it, but with no success. I will use the "new branch" option to do it. Thanks. https://codereview.appspot.com/6844061/ -- https://code.launchpad.net/~tveronezi/juju-gui/make-build/+merge/134706 Your team Juju GUI Hackers is requested to review the proposed merge of lp:~tveronezi/juju-gui/make-build 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
