Jay Soffian <jaysoffian@xxxxxxxxx> writes:
> Thoughts?
I am personally not very interested in this particular "author, timestamp,
title and nothing else" implementation, as that is probably too loose (in
many projects, title by itself is not descriptive enough) to be safe.
Also people would probably want other loose modes with varying degree
(e.g. throwing in the list of touched paths to your mix might make it a
bit safer), so "loose" feels a bit too broad a word to give to this
particular implementation (iow, it does not say in what way it is loose).
> @@ -65,8 +79,13 @@ static struct patch_id *add_commit(struct commit *commit,
> unsigned char sha1[20];
> int pos;
>
> - if (commit_patch_id(commit, &ids->diffopts, sha1))
> - return NULL;
> + if (ids->loose) {
> + if (commit_patch_id_loose(commit, sha1))
> + return NULL;
> + } else {
> + if (commit_patch_id(commit, &ids->diffopts, sha1))
> + return NULL;
> + }
If the purpose of the patch is to stir the discussion, it is fine to have
any crappy "here is a strawman" algorithm as an example of an alternative
patch ID computation, but one thing it _should_ do well is to show where
the necessary change should be hooked into, and I think the above "if"
statement is placed in a wrong function. If you change commit_patch_id()
to take a pointer to the whole ids instead of just &ids->diffopts, it can
decide how the "commit patch ID" is computed without affecting the
callers, no?
And then we could instead introduce patch-id-algo=<foo>, and instead of
"loose" call this particular algorithm "authorship-subject" or something.
Coming up with a pluggable interface to let the end user compute patch
equivalence might be a plus.
Certain patch equivalence might not be easy to define by "do they hash to
the same small value" but by "here are two patches---compare them and tell
me if they are equivalent". If we can update the code to support that
kind of patch equivalence it would be great, but it is not within the
reach/scope of this patch (not a complaint, but something you may want to
tackle next).
--
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