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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]