Re: [PATCH v4 0/8] fetch: rework negotiation tip options
- From
Derrick Stolee <stolee@gmail.com>
- Date
- May 18, 2026, 19:27 UTC
- Message-ID
- <b65e500e-ec9a-4dd9-8267-3e7843e410cd@gmail.com>
- In-Reply-To
- <VI0PR03MB116343A44C3D5E2562FBBEEEBC0032@VI0PR03MB11634.eurprd03.prod.outlook.com>
On 5/18/2026 1:24 PM, Matthew John Cheetham wrote:
Show 42 quoted lines
> On 2026-05-14 13:41, Derrick Stolee via GitGitGadget wrote: > >> Updates in v4 >> ============= >> >> Thanks, Matthew, for the detailed review! There are some big changes in this >> version. >> >> * Expanded commit message to cite the commit that introduced the bug >> (3f763ddf28). >> * Renamed --negotiation-tip to --negotiation-restrict throughout docs/code >> (including send-pack.c, transport-helper.c, builtin/pull.c). Added >> OPT_ALIAS in git-pull. >> * Switched config parsing to use parse_transport_option() helper. Removed >> git push from docs (not implemented yet). Restructured --negotiate-only >> validation flow. >> * NEW Patch 5: Added have_sent() interface to negotiators, so included >> haves can be de-duplicated properly by the negotiation algorithm. >> * Replaced COMMON flag hack with negotiator->have_sent() calls. Moved >> ref-pattern resolution into builtin/fetch.c (add_negotiation_tips()) so >> fetch-pack receives pre-resolved oid_array instead of string_list. Added >> test for --negotiation-tip ignoring missing refs. Added >> duplicate-avoidance test for v0. Accepts commit hashes in addition to ref >> names/globs. >> * Use parse_transport_option() for config. Updated docs to mention commit >> hashes. Removed git push from config docs. Fixed test to use correct >> restrict/include combinations. >> * In the last patch, add doc notes that remote config values also apply >> during git push with push.negotiate, now that they are integrated by that >> change. >> > > Thank you for going through the comments on v3 in detail. This is a nice > improvement overall. > > The main thing flagged (the COMMON bit confusion) is resolved by adding > the new have_sent() API on the negotiator interface, which is much > clearer and cleaner. The hoisting of the ref resolution to the same > layer and reuse of add_negotiate_tips() is also done and appreciated! > > I've left replies on each patch, with only a small number of easily > addressed comments.
Thanks for your review, including a confirmation that I properly responded to your earlier review. Soon, I'll send a new version with the few minor edits included.
Thanks, -Stolee