Various small cleanups

Various small cleanups

This branch has a little for everyone.

For Nicola, and for those with a masochistic enjoyment of Makefile churn, I addressed bug 1092199: "Makefile build target is misnamed."  See the bug for rationale and for the plan, which I followed except that I used "build-shared" instead of "build-common" as the new name for the former "build" directory.  This "build-shared" choice, like a couple of other choices in this branch, was fairly arbitrary.  I'd rather not change it, but that's because of time and inertia.  If I'd reread the bug before starting I probably would have used build-common.  Fixes for this bug account for the vast majority of the Makefile changes.  The two exceptions are the change to the JSFILES variable definition, and the removal of the skin asset Make dependencies for build-prod and build-devel.  I'll explain those in a moment.  This bug also accounts for many of the changes in bin/merge-files, and various other places where you see s/build/build-shared/.

Thanks to the diagnosis and discussion from Nicola, Francesco and Kapil, I addressed one of the remaining parts of bug 1083920, "Charm should serve the GUI assets over HTTPS".  Francesco identified two insecure sources we were having trouble with when we served over HTTPS.  One insecure connection was the charm store.  Kapil is working on getting a cert for the charm store, so we can simply use HTTPS for our charm store URLs.  The other insecure connection is that we were still relying on a number of files from the Yahoo CDN.  In prod, we relied on the HTTP-only CDN for three Gallery files, and in debug (and devel) we relied on the CDN for many files in addition.  I fixed the way we serve our files in devel, debug and prod, so that all YUI files are served locally.  The main step for this was to get the gallery files stored locally.  I could have done this many ways.  One choice could have been to use the npm yui-gallery package, but it is quite old so I rejected it.  I also could have gotten all the gallery files from git as part of the build process.  That would have been fine, as would have other approaches, but I went for an explicit and simple approach: I downloaded the files we needed into app/assets/javascripts and told the debug modules file where to find them and told the minifier to include them.  This also required some tweaks to the Makefile's JSFILES calculation, to exclude those files.

With help from Benji, I updated the reviewer documentation and the release documentation to reflect the current state of things.  Hopefully those changes are self-explanatory, and if they are not, we should probably fix them so they are!

I removed some unnecessarily duplicated and ignored topology "requires" in the modules-debug.js file, as requested by a comment at the top of that file and as blessed by Ben.

More valuably, I seem to have fixed the errors we saw in ``make test-prod`` in Ben's recently landed branch.  I believe it was because of the fake console missing a "debug" call.  In any case, I was having trouble with prod, and now it is working for me, and the prod tests seem fine too.

I tweaked Benji's Makefile change that had switched back from linking to copying YUI files into the build directories.  The old symlinks had put the skin file assets in the wrong place.  When Benji fixed the assets, he didn't notice that the Makefile said that it depended on the wrong location of those files.  This meant that every time you ran make, the Makefile would rerun the linking code, in the vain attempt to create those incorrectly-placed files.  Rather than include all skin asset locations, I simply removed those two dependencies ("night" and "sam").  Now the Makefile is as quiet as desired on repeated runs of "make," "make prod," "make debug," and "make devel."

https://code.launchpad.net/~gary/juju-gui/grabbag/+merge/141022

(do not edit description out of merge proposal)

[gary.poster, 2012-12-21 02:37:16]

Please take a look.

[frankban, 2012-12-21 10:57:51]

Land with changes.

This review is done by me and Nicola.
This branch looks great Gary, it solves a lot of problems.
The only one issue we found is the presence of failures in test-debug, but below
we are suggesting a solution.
Thanks also for the documentation clean up (especially the how to make releases
part).
Nicola says: special thanks for renaming the build-devel target, you da man! :-)

https://codereview.appspot.com/7005044/diff/1/Makefile
File Makefile (left):

https://codereview.appspot.com/7005044/diff/1/Makefile#oldcode455
Makefile:455: .PHONY: appcache-force appcache-touch beautify build \
Isn't build a phony target now?

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

https://codereview.appspot.com/7005044/diff/1/Makefile#newcode32
Makefile:32: -e '^app/assets/javascripts/gallery-.*\.js$$' \
Really nice, thank you.

https://codereview.appspot.com/7005044/diff/1/app/modules-debug.js
File app/modules-debug.js (right):

https://codereview.appspot.com/7005044/diff/1/app/modules-debug.js#newcode29
app/modules-debug.js:29: fullpath:
'juju-ui/assets/javascripts/gallery-timer-debug.js'
These three paths need to be absolute (add a slash at the beginning of each
one). Otherwise, in test-debug runs, they are searched inside the /test/ path,
and a bunch of tests fail.

https://codereview.appspot.com/7005044/diff/1/app/views/utils.js
File app/views/utils.js (right):

https://codereview.appspot.com/7005044/diff/1/app/views/utils.js#newcode121
app/views/utils.js:121: debug: noop
Yay! No more errors running test-prod!

[gary.poster, 2012-12-21 12:56:14]

*** Submitted:

Various small cleanups

This branch has a little for everyone.

For Nicola, and for those with a masochistic enjoyment of Makefile churn, I
addressed bug 1092199: "Makefile build target is misnamed."  See the bug for
rationale and for the plan, which I followed except that I used "build-shared"
instead of "build-common" as the new name for the former "build" directory. 
This "build-shared" choice, like a couple of other choices in this branch, was
fairly arbitrary.  I'd rather not change it, but that's because of time and
inertia.  If I'd reread the bug before starting I probably would have used
build-common.  Fixes for this bug account for the vast majority of the Makefile
changes.  The two exceptions are the change to the JSFILES variable definition,
and the removal of the skin asset Make dependencies for build-prod and
build-devel.  I'll explain those in a moment.  This bug also accounts for many
of the changes in bin/merge-files, and various other places where you see
s/build/build-shared/.

Thanks to the diagnosis and discussion from Nicola, Francesco and Kapil, I
addressed one of the remaining parts of bug 1083920, "Charm should serve the GUI
assets over HTTPS".  Francesco identified two insecure sources we were having
trouble with when we served over HTTPS.  One insecure connection was the charm
store.  Kapil is working on getting a cert for the charm store, so we can simply
use HTTPS for our charm store URLs.  The other insecure connection is that we
were still relying on a number of files from the Yahoo CDN.  In prod, we relied
on the HTTP-only CDN for three Gallery files, and in debug (and devel) we relied
on the CDN for many files in addition.  I fixed the way we serve our files in
devel, debug and prod, so that all YUI files are served locally.  The main step
for this was to get the gallery files stored locally.  I could have done this
many ways.  One choice could have been to use the npm yui-gallery package, but
it is quite old so I rejected it.  I also could have gotten all the gallery
files from git as part of the build process.  That would have been fine, as
would have other approaches, but I went for an explicit and simple approach: I
downloaded the files we needed into app/assets/javascripts and told the debug
modules file where to find them and told the minifier to include them.  This
also required some tweaks to the Makefile's JSFILES calculation, to exclude
those files.

With help from Benji, I updated the reviewer documentation and the release
documentation to reflect the current state of things.  Hopefully those changes
are self-explanatory, and if they are not, we should probably fix them so they
are!

I removed some unnecessarily duplicated and ignored topology "requires" in the
modules-debug.js file, as requested by a comment at the top of that file and as
blessed by Ben.

More valuably, I seem to have fixed the errors we saw in ``make test-prod`` in
Ben's recently landed branch.  I believe it was because of the fake console
missing a "debug" call.  In any case, I was having trouble with prod, and now it
is working for me, and the prod tests seem fine too.

I tweaked Benji's Makefile change that had switched back from linking to copying
YUI files into the build directories.  The old symlinks had put the skin file
assets in the wrong place.  When Benji fixed the assets, he didn't notice that
the Makefile said that it depended on the wrong location of those files.  This
meant that every time you ran make, the Makefile would rerun the linking code,
in the vain attempt to create those incorrectly-placed files.  Rather than
include all skin asset locations, I simply removed those two dependencies
("night" and "sam").  Now the Makefile is as quiet as desired on repeated runs
of "make," "make prod," "make debug," and "make devel."

R=frankban
CC=
https://codereview.appspot.com/7005044

https://codereview.appspot.com/7005044/diff/1/Makefile
File Makefile (left):

https://codereview.appspot.com/7005044/diff/1/Makefile#oldcode455
Makefile:455: .PHONY: appcache-force appcache-touch beautify build \
On 2012/12/21 10:57:51, frankban wrote:
> Isn't build a phony target now?

Yes, good catch, thank you.

[gary.poster, 2012-12-21 13:13:58]

Thank you, Francesco and Nicola!

https://codereview.appspot.com/7005044/diff/1/app/modules-debug.js
File app/modules-debug.js (right):

https://codereview.appspot.com/7005044/diff/1/app/modules-debug.js#newcode29
app/modules-debug.js:29: fullpath:
'juju-ui/assets/javascripts/gallery-timer-debug.js'
On 2012/12/21 10:57:51, frankban wrote:
> These three paths need to be absolute (add a slash at the beginning of each
> one). Otherwise, in test-debug runs, they are searched inside the /test/ path,
> and a bunch of tests fail.

Thank you!  Fixed.

https://codereview.appspot.com/7005044/diff/1/app/views/utils.js
File app/views/utils.js (right):

https://codereview.appspot.com/7005044/diff/1/app/views/utils.js#newcode121
app/views/utils.js:121: debug: noop
On 2012/12/21 10:57:51, frankban wrote:
> Yay! No more errors running test-prod!

:-)