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. -- Mailing list: https://launchpad.net/~yellow Post to : [email protected] Unsubscribe : https://launchpad.net/~yellow More help : https://help.launchpad.net/ListHelp
