Re: [PATCH v3 02/12] rerere: convert flags into an enum

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> +enum rerere_flags {
> +	/*
> +	 * Auto-update the index with previously recorded resolutions. This can
> +	 * be used to explicitly override "rerere.autoUpdate".
> +	 */
> +	RERERE_AUTOUPDATE = (1 << 0),
> +
> +	/*
> +	 * Reverse of RERERE_AUTOUPDATE. That is, update the file in the
> +	 * working tree but don't touch the index.
> +	 */
> +	RERERE_NOAUTOUPDATE = (1 << 1),

As you have updated the comment in this iteration, I'll throw a bit
of review in.  The new comment is not wrong per-se, but the one from
the previous iteration was sufficient to convey the same thing.  In
other words, I see no improvement in v3 over what you had in v2 for
this bit.

What BOTH versions lack is an explanation why RERERE_NOAUTOUPDATE is
needed in the first place.  In other words, "How is it different
from the lack of RERERE_AUTOUPDATE bit?" is not answered here.  And
I think that is a lot more important thing to clarify.

> +	/* Skip acquiring the MERGE_RR.lock for a read-only operation. */
> +	RERERE_READONLY = (1 << 2),
> +
> +	/* If MERGE_RR.lock is taken, return -1 as if rerere were disabled. */
> +	RERERE_NOWAIT = (1 << 3),
> +};