Re: [PATCH] git-instaweb: Add option to reuse previous config file

[Petr Baudis <pasky@suse.cz>]

On Tue, Jun 01, 2010 at 07:14:05PM +0530, Pavan Kumar Sunkara wrote:
> @@ -551,7 +557,7 @@ our \$projects_list = \$projectroot;
>  EOF
>  }
>  
> -gitweb_conf
> +test "$no_reuse" = true && gitweb_conf
>  
>  resolve_full_httpd
>  mkdir -p "$fqgitdir/gitweb/$httpd_only"

It is a better style to write:

	test "$no_reuse" = false || gitweb_conf

The reason is that the script can then be easily run with set -e. But I
know that the rest of git-instaweb doesn't really follow this
convention, so this is just a nitpick. (Moreover, why do you have option
--reuse-config but variable no_reuse with an oppposite meaning?)

On Tue, Jun 01, 2010 at 11:31:37PM +0200, Jakub Narebski wrote:
> If git-instaweb is invoked *without* --reuse-config, the gitweb_config.perl
> would be regenerated whether it exists or not, overwriting your changes.
> 
> If git-instaweb is invoked *with* --reuse-config, the gitweb_config.perl
> would be generated if it does not exist (so if you delete gitweb_config.perl
> and then run 'git instaweb --reuse-config' it would not fail), and reused
> if it does exist.

That sounds like a good idea, but I think at that point --reuse-config
gets to be a way too misleading name. --keep-config might be tiny bit
better, but not a lot.

>   test "$no_reuse" = true || test ! -e "$GITWEB_CONFIG" && gitweb_conf
> 
> or something like that (the test if $GITWEB_CONFIG file exists might be
> moved to the fragment of code that sets no_reuse to true instead).

BTW, I think at the point you have to chain more than two pieces at once
in the boolean sequence, it gets clearer to write an if. Just my
personal opinion, though. (Mainly because I can never remember the
priorities. ;-)


-- 
				Petr "Pasky" Baudis
The true meaning of life is to plant a tree under whose shade
you will never sit.