On Thu, Jul 13, 2017 at 2:23 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Stefan Beller <sbeller@xxxxxxxxxx> writes: > >> On Thu, Jul 13, 2017 at 1:52 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: >>> Stefan Beller <sbeller@xxxxxxxxxx> writes: >>> >>>> diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c >>>> index f9a5f7535a..ed8329340f 100644 >>>> --- a/builtin/verify-tag.c >>>> +++ b/builtin/verify-tag.c >>>> @@ -56,20 +56,21 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) >>>> } >>>> >>>> while (i < argc) { >>>> - unsigned char sha1[20]; >>>> + struct object_id oid; >>>> const char *name = argv[i++]; >>>> - if (get_sha1(name, sha1)) { >>>> + >>>> + if (get_oid(name, &oid)) { >>>> had_error = !!error("tag '%s' not found.", name); >>>> continue; >>>> } >>> >>> This part is already done, it seems, in bc/object-id topic, even >>> though other parts are not yet done? >> >> Oops. I assumed the latest bc/object-id would have been in master >> already, but after checking it is not. 967635dc3c2 >> (builtin/verify-tag: convert to struct object_id) >> converts this part, although there are 2 differences: >> * I added a stray newline before get_oid >> * The argument to gpg_verify_tag is a sha1 or oid >> >> So yes, this produces a merge conflict. :/ > > That is OK. This actually shouldn't create any meaningful conflict. > Both try to do the same code, with only a blank-line difference. > > As Brian said bc/object-id would be rerolled, I was wondering if I > should queue these two patches (even though I already queued them) > myself, or it would be better for you to send them to Brian to make > it part of his series. +cc Brian Snarkily I wanted to link to an essay "large patch series considered harmful"[1], but could not find any. So a couple of bullet points: (a) humans dislike larger series, hence fewer reviewers (b) the likelihood of a mistake in new code is proportional to its size We can use the number of patches as a proxy for size (c) If a mistake is found, the whole series needs rerolling. The effort of rerolling a series can be approximated with its size as well. >From combining (b) and (c), we conclude that the effort to land a patch series is O(n^2) with n as the number of patches. Also from (a) we conclude that two smaller series containing the same output as one large series, has better code quality. So with that, we conclude that all series shall be as small as possible. So I'd ask to queue these 2 separately, asking Brian to drop "builtin/verify-tag: convert to struct object_id" [1] https://www.cs.princeton.edu/~dpw/papers/hotos.pdf, 2005 seems interesting to me in hindsight. I can also send my patches to Brian, as you (both) like. Thanks, Stefan