https://codereview.appspot.com/6821090/diff/15005/Makefile File Makefile (right): https://codereview.appspot.com/6821090/diff/15005/Makefile#newcode82 Makefile:82: @./bin/write-properties true On 2012/11/09 13:57:38, gary.poster wrote: > I really would like to see this (and the corresponding line in 87 and the > corresponding "write-properties" script) go. > My preferred approach, which we talked about yesterday, would be to make the > server always have both the debug version (debug.html?) and the production > version (index.html) available, by having the server dynamically change the js > files that index.html points to. > Alternatively, server.js would accept an argument to turn on debug mode. By > default, it would run in prodcution mode. Using the server approach for now. Thanks for this idea. Really good. https://codereview.appspot.com/6821090/diff/15005/app/modules-debug.js File app/modules-debug.js (right): https://codereview.appspot.com/6821090/diff/15005/app/modules-debug.js#newcode1 app/modules-debug.js:1: GlobalConfig = { On 2012/11/09 13:57:38, gary.poster wrote: > An idea (as in, do this if you agree): it would be nice to have a comment at the > top of this file saying that it is only used for the debug (developer) views of > the application, and pointing to the mechanism that is used (e.g., in server.js) > to serve this file rather than the other one. > Maybe it's overkill. I'd do it, but I'd be fine with accepting disagreement. Done. https://codereview.appspot.com/6821090/diff/15005/bin/merge-files File bin/merge-files (right): https://codereview.appspot.com/6821090/diff/15005/bin/merge-files#newcode1 bin/merge-files:1: #!/usr/bin/env node On 2012/11/09 13:57:38, gary.poster wrote: > These changes look great to me. Thanks. Done. https://codereview.appspot.com/6821090/diff/15005/bin/merge-files#newcode4 bin/merge-files:4: * We should aggregate and minimize the js sources in order to improve the load On 2012/11/09 13:57:38, gary.poster wrote: > suggestion: delete "should". (We are doing it. :-) ) Done. https://codereview.appspot.com/6821090/diff/15005/bin/merge-files#newcode8 bin/merge-files:8: * to run from only static files and we want to be able to run behaind a On 2012/11/09 13:57:38, gary.poster wrote: > "behind" (I realize these are probably my fault. sorry :-) ) Done. https://codereview.appspot.com/6821090/diff/15005/bin/write-properties File bin/write-properties (right): https://codereview.appspot.com/6821090/diff/15005/bin/write-properties#newcode1 bin/write-properties:1: #!/usr/bin/env node On 2012/11/09 13:57:38, gary.poster wrote: > As I said, I'd like to delete. Done. https://codereview.appspot.com/6821090/diff/15005/lib/merge-files.js File lib/merge-files.js (right): https://codereview.appspot.com/6821090/diff/15005/lib/merge-files.js#newcode14 lib/merge-files.js:14: // end. On 2012/11/09 13:57:38, gary.poster wrote: > Another option (up to you): remove this comment, and do the export of each > function as you define it ("expose.minify = function(file) {...}" for instance) > or immediately after you define it. Done. https://codereview.appspot.com/6821090/diff/15005/lib/merge-files.js#newcode29 lib/merge-files.js:29: // or not) On 2012/11/09 13:57:38, gary.poster wrote: > Trivial: Please adjust the comment formatting. My formatter formats comments... code and formatter fixed. https://codereview.appspot.com/6821090/diff/15005/lib/merge-files.js#newcode44 lib/merge-files.js:44: // ignores On 2012/11/09 13:57:38, gary.poster wrote: > Trivial: Please adjust the comment formatting. Done. https://codereview.appspot.com/6821090/diff/15005/lib/merge-files.js#newcode75 lib/merge-files.js:75: // it is On 2012/11/09 13:57:38, gary.poster wrote: > Trivial: Please adjust the comment formatting. Done. https://codereview.appspot.com/6821090/diff/15005/lib/merge-files.js#newcode86 lib/merge-files.js:86: // (as On 2012/11/09 13:57:38, gary.poster wrote: > Trivial: Please adjust the comment formatting. Done. https://codereview.appspot.com/6821090/diff/15005/lib/merge-files.js#newcode128 lib/merge-files.js:128: // these On 2012/11/09 13:57:38, gary.poster wrote: > Trivial: Please adjust the comment formatting. Done. https://codereview.appspot.com/6821090/