From: Junio C Hamano <gitster@pobox.com>
To: Glen Choo <chooglen@google.com>
Cc: git@vger.kernel.org, Jonathan Tan <jonathantanmy@google.com>
Subject: Re: [PATCH v2] builtin/fetch: skip unnecessary tasks when using --negotiate-only
Date: Mon, 20 Dec 2021 11:56:30 -0800 [thread overview]
Message-ID: <xmqq4k73vymp.fsf@gitster.g> (raw)
In-Reply-To: <kl6ltuf3ysnw.fsf@chooglen-macbookpro.roam.corp.google.com> (Glen Choo's message of "Mon, 20 Dec 2021 11:37:07 -0800")
Glen Choo <chooglen@google.com> writes:
>> By the way, do not move the check about the number of negotiation
>> tips from the original location. That check, or its location, have
>> nothing to do with what you want to do in this patch, which is "do
>> not gc or update the graph file if we are not fetching". It is
>> better to leave unrelated changes out of the patch.
>
> Ah, I see that it's not easy to tell whether or not the behavior is
> correct after that line is moved. I'll avoid doing this in the future.
>
> I still think that it is cleaner to move the negotiation_tip.nr check.
> Should I do this in a follow-up patch?
I am not seeing what makes it cleaner to have it early or at the
current position, but with "It is cleaner to do tip.nr check early
because ...", in a separate patch, it may become obvious. But let's
not do it in this patch.
> I hope Jonathan can chime in to confirm whether or not users want/need
> to invoke "--negotiate-only".
Yeah, I knew that this was "internal implementation detail", but
then perhaps we know that a sane developer who knows what they are
doing will never write combination of it with recurse-submodule
option? If so, we'd catch a notice developer breaking the system by
having a sanity check by detecting it as an error, no?
next prev parent reply other threads:[~2021-12-20 19:56 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-07 19:29 [PATCH] builtin/fetch: skip unnecessary tasks when using --negotiate-only Glen Choo
2021-12-09 22:12 ` Jonathan Tan
2021-12-09 22:36 ` Glen Choo
2021-12-13 22:58 ` Jonathan Tan
2021-12-16 18:11 ` Glen Choo
2021-12-17 0:02 ` [PATCH v2] " Glen Choo
2021-12-17 23:35 ` Junio C Hamano
2021-12-20 19:37 ` Glen Choo
2021-12-20 19:56 ` Junio C Hamano [this message]
2021-12-20 20:54 ` Glen Choo
2021-12-20 22:12 ` Junio C Hamano
2021-12-21 0:18 ` Glen Choo
2021-12-21 23:07 ` Glen Choo
2021-12-22 0:11 ` [PATCH v3 0/3] " Glen Choo
2021-12-22 0:11 ` [PATCH v3 1/3] builtin/fetch: use goto cleanup in cmd_fetch() Glen Choo
2021-12-22 0:11 ` [PATCH v3 2/3] builtin/fetch: skip unnecessary tasks when using --negotiate-only Glen Choo
2021-12-22 6:42 ` Junio C Hamano
2021-12-22 17:28 ` Glen Choo
2021-12-22 19:29 ` Junio C Hamano
2021-12-22 20:27 ` Glen Choo
2021-12-22 0:11 ` [PATCH v3 3/3] builtin/fetch: die on --negotiate-only and --recurse-submodules Glen Choo
2021-12-22 6:46 ` Junio C Hamano
2021-12-23 19:08 ` Jonathan Tan
2022-01-13 0:44 ` [PATCH v4 0/3] fetch: skip unnecessary tasks when using --negotiate-only Glen Choo
2022-01-13 0:44 ` [PATCH v4 1/3] fetch: use goto cleanup in cmd_fetch() Glen Choo
2022-01-13 0:45 ` [PATCH v4 2/3] fetch: skip tasks related to fetching objects Glen Choo
2022-01-13 0:45 ` [PATCH v4 3/3] fetch --negotiate-only: do not update submodules Glen Choo
2022-01-13 1:16 ` Junio C Hamano
2022-01-18 18:54 ` [PATCH v5 0/3] fetch: skip unnecessary tasks when using --negotiate-only Glen Choo
2022-01-18 18:54 ` [PATCH v5 1/3] fetch: use goto cleanup in cmd_fetch() Glen Choo
2022-01-18 18:54 ` [PATCH v5 2/3] fetch: skip tasks related to fetching objects Glen Choo
2022-01-18 18:54 ` [PATCH v5 3/3] fetch --negotiate-only: do not update submodules Glen Choo
2022-01-18 22:05 ` Junio C Hamano
2022-01-18 23:41 ` Glen Choo
2022-01-19 0:26 ` Junio C Hamano
2022-01-19 0:00 ` [PATCH v6 0/3] fetch: skip unnecessary tasks when using --negotiate-only Glen Choo
2022-01-19 0:00 ` [PATCH v6 1/3] fetch: use goto cleanup in cmd_fetch() Glen Choo
2022-01-19 0:00 ` [PATCH v6 2/3] fetch: skip tasks related to fetching objects Glen Choo
2022-01-19 0:00 ` [PATCH v6 3/3] fetch --negotiate-only: do not update submodules Glen Choo
2022-01-20 2:38 ` Jiang Xin
2022-01-20 17:40 ` Glen Choo
2022-01-20 17:49 ` [PATCH v7 0/3] fetch: skip unnecessary tasks when using --negotiate-only Glen Choo
2022-01-20 17:49 ` [PATCH v7 1/3] fetch: use goto cleanup in cmd_fetch() Glen Choo
2022-01-20 17:49 ` [PATCH v7 2/3] fetch: skip tasks related to fetching objects Glen Choo
2022-01-20 17:49 ` [PATCH v7 3/3] fetch --negotiate-only: do not update submodules Glen Choo
2022-01-20 23:08 ` Junio C Hamano
2022-01-20 23:16 ` Glen Choo
2022-01-20 21:58 ` Re* [PATCH v7 0/3] fetch: skip unnecessary tasks when using --negotiate-only Junio C Hamano
2022-01-20 23:15 ` Glen Choo
2022-01-21 2:17 ` Jiang Xin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq4k73vymp.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=chooglen@google.com \
--cc=git@vger.kernel.org \
--cc=jonathantanmy@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.