Subject: Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css

From: Jakub Narebski <[email-masked]>
Date: Thu, 1 Apr 2010 10:51:38 +0200

On Thu, 1 April 2010, Mark Rada wrote:

> The build system added support minifying gitweb.js through a
> JavaScript minifier, but most minifiers come with support to
> minify CSS files as well, so we should use it if we can.
> 
> This patch will add the same facilities to gitweb.css that
> gitweb.js has for minification. That does not mean that they
> will use the same minifier though, as it is not safe to assume
> that all JavaScript minifiers will also minify CSS files.
> 
> Though the bandwidth savings will not be as dramatic as with
> the JavaScript minifier, every byte saved is important.

That's not everything this patch does, isn't it?  I think you should
have added to your commit message the following paragraph:

  While at it, introduce GITWEB_PROGRAMS variable to Makefile and
  use it for gitweb instead of adding to OTHER_PROGRAMS, to keep
  (possible) gitweb dependencies separate.

Or something like that.

> 
> Signed-off-by: Mark Rada <[email-masked]>

I think it is good idea.

[...]
>  ifdef JSMIN
> -OTHER_PROGRAMS += gitweb/gitweb.cgi   gitweb/gitweb.min.js
> -gitweb/gitweb.cgi: gitweb/gitweb.perl gitweb/gitweb.min.js
> -else
> -OTHER_PROGRAMS += gitweb/gitweb.cgi
> -gitweb/gitweb.cgi: gitweb/gitweb.perl
> +GITWEB_PROGRAMS += gitweb/gitweb.min.js
>  endif
> +ifdef CSSMIN
> +GITWEB_PROGRAMS += gitweb/gitweb.min.css
> +endif
> +OTHER_PROGRAMS +=  gitweb/gitweb.cgi  $(GITWEB_PROGRAMS)
> +gitweb/gitweb.cgi: gitweb/gitweb.perl $(GITWEB_PROGRAMS)
>  	$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) $(patsubst gitweb/%,%,$@)

-- 
Jakub Narebski
Poland