On Mon, 6 June 2011, Jonathan Nieder wrote:
Thank you very much for your review and comments.
> Jakub Narebski wrote:
>
> > The build-time configuration variables JSMIN and CSSMIN were mentioned
> > only in Makefile; add their description to gitweb/INSTALL.
> >
> > This required moving description of GITWEB_JS up, near GITWEB_CSS and
> > just introduced CSMIN and JSMIN.
>
> Why does this require that? Ah, to make the analogy with GITWEB_CSS
> clear and because the JSMIN description refers to it.
Yes, moving description of GITWEB_JS was required because I wanted to
cover both of CSSMIN and JSMIN with one entry, which entry of course
refers to both GITWEB_CSS and GITWEB_JS.
> > --- a/gitweb/INSTALL
> > +++ b/gitweb/INSTALL
> > @@ -147,6 +147,19 @@ You can specify the following configuration variables when building GIT:
> [...]
> > + * CSSMIN, JSMIN
> > + Invocation of a CSS minifier or a JavaScript minifier, respectively,
> > + working as a filter (source on standard input, minified result on
> > + standard output). If set, it is used to generate a minified version of
> > + 'static/gitweb.css' or 'static/gitweb.js', respectively. *Note* that
> > + minified files would have *.min.css and *.min.js extension, which is
> > + important if you also set GITWEB_CSS and/or GITWEB_JS. [No default]
>
> When I first read this, I thought it meant these command lines were
> going to be cooked into the gitweb script and invoked at run time.
> Maybe (sorry for the rough text):
>
> * CSSMIN, JSMIN
> Command for a CSS minifier or a Javascript minifier, working as a
> filter [...]
> These are used if defined to generate smaller, non human-readable
> versions of 'gitweb/gitweb.css' and 'static/gitweb.js' at
> 'static/gitweb.min.css' and 'static/gitweb.min.js'. Only the minified
> versions are installed, which is important if you also set GITWEB_CSS
> or GITWEB_JS. [No default]
>
> Aside from that, looks good. Thanks.
Thanks. I'll incorporate those comments in next round... or as a separate
improvement (it is not that bad, I think, to not allow it to be fixed
"in tree").
Anyway the description could use some improvements. We need to cover the
following issues:
1. CSSMIN and JSMIN are invoked during building gitweb.
2. CSSMIN and JSMIN are interpreted as shell commands, so
* if you refer to script by full path, you need to quote spaces
yourself, e.g.
JSMIN="'c:/Program Files/JSMin/jsmin.exe'"
* you can provide options, so you can e.g. use
JSMIN="perl -MJavaScript::Minifier=minify -we 'minify(input => *STDIN, output => *STDOUT);'"
3. CSSMIN and JSMIN must accept source on standard input, and print
minified version to standard output, i.e. they are invoked as
$(CSSMIN) <$< >$@
4. Minified files will be named static/gitweb.min.css and
static/gitweb.min.js, respectively
5. If you do not set GITWEB_CSS and GITWEB_JS, but use CSS and/or JSMIN,
they would be set to appropriate values automatically
6. The "install" target in gitweb Makefile, and "install-gitweb" target
in main Makefile, would install minified versions (if any).
7. If you set both CSSMIN and GITWEB_CSS, you have to adjust GITWEB_CSS
by yourself. Same for JSMIN and GITWEB_JS.
Help with wording those would be much appreciated, in more compact form.
Thanks in advance.
--
Jakub Narebski
Poland (not native English speaker)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html