yellow team mailing list archive

[Gary Poster <gary.poster@xxxxxxxxxxxxx>]

Land with changes

Looks good!  Thank you.  This will be great to have.


https://codereview.appspot.com/6873071/diff/1/Makefile
File Makefile (right):

https://codereview.appspot.com/6873071/diff/1/Makefile#newcode13
Makefile:13: JSFILES=$(shell find . -wholename './node_modules*' -prune
\
Before this, I think we need a comment explaining the two use cases for
not using bzr commands: we want to make releases without bzr access, so
working with a branch is fast; and we want to allow devs to work with a
release (per Kapil; I really don't like this.  devs should work with a
branch.)

https://codereview.appspot.com/6873071/diff/1/Makefile#newcode19
Makefile:19: -o -name '*generateTemplates' \
This could simply be 'generateTemplates', without the wildcard.

https://codereview.appspot.com/6873071/diff/1/Makefile#newcode40
Makefile:40: ifdef NO_BZR
Good solution, given our use cases and the other problems you
encountered.

https://codereview.appspot.com/6873071/diff/1/Makefile#newcode102
Makefile:102: TEMPLATE_TARGETS=$(shell find app/templates -type f
-print)
Please exclude standard editor effluvia.

https://codereview.appspot.com/6873071/diff/1/Makefile#newcode104
Makefile:104: SPRITE_SOURCE_FILES=$(shell find app/assets/images -type f
-print)
Please exclude standard editor effluvia.

https://codereview.appspot.com/6873071/diff/1/Makefile#newcode393
Makefile:393: dist: $(RELEASE_FILE) $(RELEASE_SIGNATURE)
upload_release.py
I would like the Makefile to refuse to run the upload_release.py file if
NO_BZR is defined.  We should then have a phony target that only creates
the release file (e.g. "distfile: $(RELEASE_FILE)" that the charm can
use (that is, it will run fine if NO_BZR is true).  See "ifdef
BRANCH_IS_GOOD" above for an example of one way to do this kind of
thing.

https://codereview.appspot.com/6873071/diff/1/docs/process.rst
File docs/process.rst (right):

https://codereview.appspot.com/6873071/diff/1/docs/process.rst#newcode214
docs/process.rst:214: all targets.
...except dist, which will refuse to complete.

https://codereview.appspot.com/6873071/

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