yellow team mailing list archive

[Gary Poster <gary.poster@xxxxxxxxxxxxx>]

Looks good to me.  After changes requested, please land!

Thank you.

Gary

=== modified file 'lib/templates.js'
--- lib/templates.js	2012-11-27 22:18:41 +0000
+++ lib/templates.js	2012-11-27 22:43:52 +0000
@@ -131,7 +131,7 @@
   * @param {function} cb Callback to call if the file is valid.
   */
  function ifValidFile(fullpath, cb) {
-  exec('bzr file-id ' + fullpath + ' > /dev/null 2>&1',
+  exec('bzr file-id ' + fullpath,
        function(error, stdout, stderr) {
          if (error === null) {
            cb();
@@ -190,7 +190,7 @@
  function watchTemplates(cb) {
    fs.watch(config.server.template_dir, function(event, filename) {
      //on dir change regen the cache
-    ifValidFile(path.join(config.server.view_dir, filename), function()
{
+    ifValidFile(path.join(config.server.template_dir, filename),
function() {
        var strategy = templateSpecs.templates;
        strategy.callback(strategy, 'templates');
        if (cb) {cb();}



https://codereview.appspot.com/6842104/diff/3002/lib/templates.js
File lib/templates.js (right):

https://codereview.appspot.com/6842104/diff/3002/lib/templates.js#newcode128
lib/templates.js:128: * are files created by vim and emacs; feel free to
add to this list.
You'll need to adjust this comment, of course.

https://codereview.appspot.com/6842104/diff/3002/lib/templates.js#newcode133
lib/templates.js:133: function ifValidFile(fullpath, cb) {
Nice naming

https://codereview.appspot.com/6842104/diff/3002/lib/templates.js#newcode134
lib/templates.js:134: exec('bzr file-id ' + fullpath + ' > /dev/null
2>&1',
you can leave out the /dev/null bit, for cleanliness.  The fact that we
ignore stdout and stderr in the callback below should be effectively
equivalent, unless the command generates so much blather that we run out
of memory or something.

https://codereview.appspot.com/6842104/diff/3002/lib/templates.js#newcode147
lib/templates.js:147: return true;*/
(You clarified on IRC that you will remove this bit if we go down this
path)

https://codereview.appspot.com/6842104/diff/3002/lib/templates.js#newcode193
lib/templates.js:193: ifValidFile(path.join(config.server.view_dir,
filename), function() {
config.server.template_dir

https://codereview.appspot.com/6842104/

-- 
https://code.launchpad.net/~makyo/juju-gui/editor-files/+merge/136484
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~makyo/juju-gui/editor-files into lp:juju-gui.