git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/8] refs.c: upgrade for_each_replace_ref to be a each_repo_ref_fn callback

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 18, 2018, 20:13 UTC
Message-ID
<xmqq4lgwjoa4.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<CAGZ79kZ9oeqidNMppOx+PNfp-4RdDhLzvQHhm45Ceni=g1Q_7Q@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
Show 25 quoted lines
>>  - existing users of for_each_replace_ref() who were all happy
>>    working in the_repository have to pass it explicitly, even
>>    thought they do not have any need to.
>
> All callbacks that are passed to for_each_replace_ref now
> operate on 'r' instead of the_repository, and that actually fixes
> a bug (commit message is lacking), but the cover letter hints at:
> ...
>> In this case, even if you introduced for_each_replace_ref_in_repo(),
>> making for_each_replace_ref() a thin wrapper that always uses
>> the_repository is a bit more cumbersome than just a simple macro.
>
> Yes, but such a callback would do the *wrong* subtle thing in some cases
> as you really want to work in the correct repository for e.g. looking up
> commits.
>
>> But it *is* doable (you'd need to use a wrapping structure around
>> cb_data), and a developer who case about maintainability during API
>> transition would have taken pains to do so.  A bit dissapointing.
>
> My original patches were RFC-ish and a trade off as for the reflog only
> there is nothing in flight to care about.
>
> Given that we would want to upgrade all the ref callbacks, we have to
> take this route, I think.

Try to rebuild 'pu' on top of 'master' yoruself to realize how many in-flight topics you are hurting and causing unnecessary load on the maintainer with the "let's add the 'r' parameter without changing the function names; compiler would catch and cause breakages" approach.

Until that happens, I won't waste any more time trying to educate you on this further.

Previous: Stefan BellerNext: Derrick Stolee via GitGitGadget
Message 6 of 47 in “Clarify commit-graph and grafts/replace/shallow incompatibilities”
  1. 0/8 Clarify commit-graph and grafts/replace/shallow incompatibilitiesDerrick Stolee via GitGitGadget, Jul 18, 2018
  2. 1/8 refs.c: migrate internal ref iteration to pass thru repository argumentStefan Beller via GitGitGadget, Jul 18, 2018
  3. 2/8 refs.c: upgrade for_each_replace_ref to be a each_repo_ref_fn callbackStefan Beller via GitGitGadget, Jul 18, 2018
  4. Junio C HamanoJul 18, 2018
  5. Stefan BellerJul 18, 2018
  6. Junio C HamanoJul 18, 2018
  7. 3/8 commit-graph: update design documentDerrick Stolee via GitGitGadget, Jul 18, 2018
  8. Jakub NarebskiJul 29, 2018
  9. 4/8 test-repository: properly init repoDerrick Stolee via GitGitGadget, Jul 18, 2018
  10. Jakub NarebskiJul 29, 2018
  11. 5/8 commit-graph: not compatible with replace objectsDerrick Stolee via GitGitGadget, Jul 18, 2018
  12. Jeff KingJul 18, 2018
  13. Jeff KingJul 18, 2018
  14. Derrick StoleeJul 18, 2018
  15. Derrick StoleeJul 20, 2018
  16. Stefan BellerJul 20, 2018
  17. Derrick StoleeJul 20, 2018
  18. Jakub NarebskiJul 29, 2018
  19. 6/8 commit-graph: not compatible with graftsDerrick Stolee via GitGitGadget, Jul 18, 2018
  20. Jakub NarebskiJul 29, 2018
  21. 7/8 commit-graph: not compatible with uninitialized repoDerrick Stolee via GitGitGadget, Jul 18, 2018
  22. Jakub NarebskiJul 29, 2018
  23. 8/8 commit-graph: close_commit_graph before shallow walkDerrick Stolee via GitGitGadget, Jul 18, 2018
  24. Jakub NarebskiJul 30, 2018
  25. DO-NOT-MERGE: write and read commit-graph alwaysDerrick Stolee, Jul 18, 2018
  26. Jakub NarebskiJul 30, 2018
  27. Stefan BellerJul 30, 2018
  28. Jakub NarebskiJul 31, 2018
  29. Elijah NewrenJul 31, 2018
  30. Jakub NarebskiJul 29, 2018
  31. Eric SunshineJul 29, 2018
  32. 0/8 Clarify commit-graph and grafts/replace/shallow incompatibilitiesDerrick Stolee, Aug 20, 2018
  33. 6/8 commit-graph: not compatible with graftsDerrick Stolee, Aug 20, 2018
  34. 7/8 commit-graph: not compatible with uninitialized repoDerrick Stolee, Aug 20, 2018
  35. 2/8 refs.c: upgrade for_each_replace_ref to be a each_repo_ref_fn callbackDerrick Stolee, Aug 20, 2018
  36. 3/8 commit-graph: update design documentDerrick Stolee, Aug 20, 2018
  37. 4/8 test-repository: properly init repoDerrick Stolee, Aug 20, 2018
  38. 5/8 commit-graph: not compatible with replace objectsDerrick Stolee, Aug 20, 2018
  39. Junio C HamanoAug 21, 2018
  40. Derrick StoleeAug 21, 2018
  41. 8/8 commit-graph: close_commit_graph before shallow walkDerrick Stolee, Aug 20, 2018
  42. 1/8 refs.c: migrate internal ref iteration to pass thru repository argumentDerrick Stolee, Aug 20, 2018
  43. Stefan BellerAug 20, 2018
  44. Junio C HamanoAug 21, 2018
  45. Derrick StoleeAug 21, 2018
  46. Stefan BellerAug 20, 2018
  47. Derrick StoleeAug 20, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.