yellow team mailing list archive

[Gary Poster <gary.poster@xxxxxxxxxxxxx>]

Looks good, Matt.  Two small suggestions.  Thanks!

Gary


https://codereview.appspot.com/6776052/diff/9005/Makefile
File Makefile (right):

https://codereview.appspot.com/6776052/diff/9005/Makefile#newcode58
Makefile:58: appcache-force: appcache-touch appcache
I'm guessing you added this because you discovered you needed it, so
that's cool if so.  That said, I don't see a reason to factor out
appcache-touch though.  Oh wait...I see.  It's an ordering issue.  OK,
new, different suggestion: add a comment explaining what is going on so
people don't have to figure it out. :-)

https://codereview.appspot.com/6776052/diff/9005/Makefile#newcode60
Makefile:60: .PHONY: test lint beautify server install clean prep jshint
gjslint appcache appcache-touch appcache-force
If you already know this, forgive me, but if you wanted to you could
make those break at 80 chars or similar by having a trailing "\" at the
end of lines that break.  Something like

.PHONY: test lint beautify server install clean prep jshint gjslint \
         appcache appcache-touch appcache-force

https://codereview.appspot.com/6776052/

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