Re: [PATCH v2 4/4] rev-list: allow missing tips with --missing=[print|allow*]
- From
- Linus Arver <linusa@google.com>
- Date
- Feb 13, 2024, 22:38 UTC
- Message-ID
- <owlyfrxw9ei3.fsf@fine.c.googlers.com>
- In-Reply-To
- <xmqqeddmonq0.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
Show 24 quoted lines
> Christian Couder <christian.couder@gmail.com> writes:
>
>> + * mechanism to allow and disallow some sets of options for
>> + * different commands (like rev-list, replay, etc). Such
>> + * mechanism should do an early parsing of option and be able
>> + * to manage the `--exclude-promisor-objects` and `--missing=...`
>> + * options below.
>> + */
>> for (i = 1; i < argc; i++) {
>> const char *arg = argv[i];
>> if (!strcmp(arg, "--exclude-promisor-objects")) {
>> @@ -753,8 +762,12 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
>>
>> if (arg_print_omitted)
>> oidset_init(&omitted_objects, DEFAULT_OIDSET_SIZE);
>> - if (arg_missing_action == MA_PRINT)
>> + if (arg_missing_action == MA_PRINT) {
>> oidset_init(&missing_objects, DEFAULT_OIDSET_SIZE);
>> + /* Already add missing tips */
>> + oidset_insert_from_set(&missing_objects, &revs.missing_commits);
>> + oidset_clear(&revs.missing_commits);
>> + }
>
> It is unclear what "already" here refers to, at least to me.It's grammatically correct but perhaps a bit "over eager" (gives the impression that we get these missing tips all the time and is a common "happy" path). I do still think my earlier v1 comments
Did you mean "Add already-missing commits"? Perhaps even more explicit
would be "Add missing tips"?are relevant here.