From: Junio C Hamano <gitster@pobox.com>
Date: Sat, 27 Feb 2010 00:07:45 GMT
Subject: Re: [PATCH] prune: honor --expire=never
Message-ID: <7v4ol3ilri.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20100226215916.GF5116@facebook.com>

Adam Simpkins <simpkins@facebook.com> writes:

> diff --git a/builtin-prune.c b/builtin-prune.c
> index 4675f60..ce43271 100644
> --- a/builtin-prune.c
> +++ b/builtin-prune.c
> @@ -7,6 +7,8 @@
>  #include "parse-options.h"
>  #include "dir.h"
>  
> +#define ALWAYS_EXPIRE ((unsigned int)-1)
> ...
> @@ -34,7 +36,7 @@ static int prune_tmp_object(const char *path, const char *filename)
>  static int prune_object(char *path, const char *filename, const unsigned char *sha1)
>  {
>  	const char *fullpath = mkpath("%s/%s", path, filename);
> -	if (expire) {
> +	if (expire != ALWAYS_EXPIRE) {

Wouldn't it be a lot simpler to initialize expire to "now" for the default
case, and remove all these "if (expire)"?  I think that is how the logic
to expire reflog entries work, which I think is saner.

While you are at it, you might want to think about a way to unify what
parse_opt_approxidate_cb() and parse_expire_cfg_value() do.  The latter
knows about "expire = false" but the former doesn't, which is a slight
inconsistency.