Re: [PATCH v2] rerere: Expose an API corresponding to 'clear' functionality

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

 



Hi,

Ramkumar Ramachandra wrote:

> Expose a new function called rerere_clear,

The commit message is a good chance to quickly explain the API.  What
does the function do?  When would one call it?  Is it equivalent to
running "git rerere clear"?  Any gotchas?

> and make 'builtin/rerere.c'
> use this when the corresponding command-line argument is passed.

Didn't "git rerere" already use the functionality of this function?
I'm not sure what this part means.

> As a
> side-effect, also expose unlink_rr_item as unlink_rerere_item.

This is not a side-effect; you did it directly.

I think the reason for this is that rerere_gc is not being exposed at
the same time, right?  I suppose if I were doing it, I would have
moved that to rerere.c, too and kept unlink_rr_item static, but there
is also appeal in a minimal patch.  It would be clearer to say
something to the effect that we

	Also export unlink_rr_item as unlink_rerere_item so
	rerere_clear and the un-libified "git rerere gc" can
	both use it.

[...]
> +++ b/builtin/rerere.c
> @@ -142,19 +134,14 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)
>  		pathspec = get_pathspec(prefix, argv + 1);
>  		return rerere_forget(pathspec);
>  	}
> +	if (!strcmp(argv[0], "clear"))
> +		return rerere_clear();
>  
>  	fd = setup_rerere(&merge_rr, flags);
[...]
> +++ b/rerere.c
> @@ -671,3 +679,22 @@ int rerere_clear(void)
[...]
> +	fd = setup_rerere(&merge_rr, RERERE_NOAUTOUPDATE);
> +	if (fd < 0)
> +		return 0;

Why RERERE_NOAUTOUPDATE instead of 0?  (Of course it doesn't matter in
practice.   Maybe "0" would convey that more clearly?)

> +
> +	for (i = 0; i < merge_rr.nr; i++) {
> +		const char *name = (const char *)merge_rr.items[i].util;
> +		if (!has_rerere_resolution(name))
> +			unlink_rerere_item(name);
> +	}
> +	string_list_clear(&merge_rr, 1);
> +	unlink_or_warn(git_path("MERGE_RR"));
> +	return 0;
> +}

The write_lock is never rolled back.  "git rerere" won't care since it
exits moments later and the atexit handler is called, but others might
mind that they can't perform any more rerere operations afterwards. :)

Thanks and hope that helps.
Jonathan
--
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]