← Back to team overview

yellow team mailing list archive

Re: Add favicon, tweak Makefile (issue 6842072)

 

Thank you, Thiago.  Replies below.  I'm happy to make a change if you
prefer, but until you tell me otherwise I'm going to proceed with the
understanding that you are fine with it as is.

Gary


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

https://codereview.appspot.com/6842072/diff/1/Makefile#newcode27
Makefile:27: test -d "$(BUILD_ASSETS_DIR)/stylesheets" || mkdir -p
"$(BUILD_ASSETS_DIR)/stylesheets"
On 2012/11/20 14:33:56, thiago wrote:
> Whats wrong with the use of "@"? Don't we like it any more? :O)
:-) "@" hides context when there are errors, and it hides the fact that
code is being re-run when it should not be in the Makefile.  IMO, the
only advantage used to be that it made restarting the server (or running
tests or whatever) quieter, but now with the changes I've made it is
pretty quiet unless it is actually doing work.  It didn't seem to get in
my way.

https://codereview.appspot.com/6842072/diff/1/Makefile#newcode42
Makefile:42: @# Check to see if we made what we expected to make, and
warn if we did not.
On 2012/11/20 14:33:56, thiago wrote:
> Do we need this @ before the comment?

If you don't, it is sent to stdout, and in this particular case I didn't
think it brought value to have it in stdout.  Maybe that's
idiosyncratic; if someone removes I'd be fine with it.

https://codereview.appspot.com/6842072/

-- 
https://code.launchpad.net/~gary/juju-gui/favicon/+merge/135045
Your team Juju GUI Hackers is subscribed to branch lp:juju-gui.


References