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

Re: [PATCH v2 1/4] diffcore-rename: compute basenames of all source and dest candidates

From
Derrick Stolee <stolee@gmail.com>
Date
Feb 9, 2021, 17:02 UTC
Message-ID
<9d8ee4be-33d7-6c50-f14b-0b92a4ebe949@gmail.com>
In-Reply-To
<CABPp-BEsuOiUyvbkwPC384eho8pgSWuRdcvw9t5gkXhf+_j-3A@mail.gmail.com>
On 2/9/2021 11:56 AM, Elijah Newren wrote:
Show 37 quoted lines
>> Also, this is used here and below. Perhaps it's worth pulling out as a
>> helper? I see similar code being duplicated in these existing spots:
>>
>> * diff-no-index.c:append_basename()
>> * help.c:append_similar_ref()
>> * packfile.c:pack_basename()
>> * replace-object.c:register_replace_ref()
>> * setup.c:read_gitfile_gently()
>> * builtin/rebase.c:cmd_rebase()
>> * builtin/stash.c:do_create_stash()
>> * builtin/worktree.c:add_worktree()
>> * contrib/credential/gnome-keyring/git-credential-gnome-keyring.c:usage()
>> * contrib/credential/libsecret/git-credential-libsecret.c:usage()
>> * trace2/tr2_dst.c:tr2_dst_try_auto_path()
> Honestly asking: would anyone ever search for such a two-line helper
> function?  I wouldn't have even thought to look, since it seems so
> simple.
> 
> However, my real concern here is that this type of change would risk
> introducing conflicts with unrelated series.  This series is the
> second in what will be a 9-series deep dependency chain of
> optimizations[1], and the later series are going to be longer than
> these first two were (the latter ones are 6-11 patches each).  We've
> already discussed previously whether we possibly want to hold the
> first couple optimization series out of the upcoming git-2.31 release
> in order to keep the optimizations all together, but that might
> increase the risk of conflicts with unrelated patches if we try a
> bigger tree refactor like this.  (Junio never commented on that,
> though.)  It might be better to keep the series touching only
> merge-ort.c & diffcore-rename.c, and then do cleanups like the one you
> suggest here after the whole series.
> 
> That said, it's not a difficult initial change, so I'm mostly
> expressing this concern out of making things harder for Junio.  It'd
> be best to get his opinion -- Junio, your thoughts?
> 
> [1] https://github.com/gitgitgadget/git/pulls?q=is%3Apr+author%3Anewren+Optimization+batch
 
I don't consider the step of "go put the helper in all these other
places" necessary for the current series. However, the "get basename"
code appears a total of three times in this series, so it would be
good to at least extract it to a static inline method to reduce
the duplication isolated to this change.

Thanks, -Stolee

Previous: Elijah NewrenNext: Elijah Newren
Message 31 of 71 in “Optimization batch 7: use file basenames to guide rename detection”
  1. 0/3 Optimization batch 7: use file basenames to guide rename detectionElijah Newren via GitGitGadget, Feb 6, 2021
  2. 1/3 diffcore-rename: compute basenames of all source and dest candidatesElijah Newren via GitGitGadget, Feb 6, 2021
  3. 2/3 diffcore-rename: complete find_basename_matches()Elijah Newren via GitGitGadget, Feb 6, 2021
  4. 3/3 diffcore-rename: guide inexact rename detection based on basenamesElijah Newren via GitGitGadget, Feb 6, 2021
  5. Derrick StoleeFeb 7, 2021
  6. Junio C HamanoFeb 7, 2021
  7. Elijah NewrenFeb 8, 2021
  8. Derrick StoleeFeb 8, 2021
  9. Elijah NewrenFeb 8, 2021
  10. Junio C HamanoFeb 8, 2021
  11. Elijah NewrenFeb 8, 2021
  12. Junio C HamanoFeb 8, 2021
  13. Elijah NewrenFeb 8, 2021
  14. Elijah NewrenFeb 8, 2021
  15. Derrick StoleeFeb 8, 2021
  16. Elijah NewrenFeb 8, 2021
  17. Junio C HamanoFeb 7, 2021
  18. Elijah NewrenFeb 7, 2021
  19. 0/4 Optimization batch 7: use file basenames to guide rename detectionElijah Newren via GitGitGadget, Feb 9, 2021
  20. 2/4 diffcore-rename: complete find_basename_matches()Elijah Newren via GitGitGadget, Feb 9, 2021
  21. Derrick StoleeFeb 9, 2021
  22. Elijah NewrenFeb 9, 2021
  23. Derrick StoleeFeb 9, 2021
  24. 3/4 diffcore-rename: guide inexact rename detection based on basenamesElijah Newren via GitGitGadget, Feb 9, 2021
  25. Derrick StoleeFeb 9, 2021
  26. Elijah NewrenFeb 9, 2021
  27. Junio C HamanoFeb 9, 2021
  28. 1/4 diffcore-rename: compute basenames of all source and dest candidatesElijah Newren via GitGitGadget, Feb 9, 2021
  29. Derrick StoleeFeb 9, 2021
  30. Elijah NewrenFeb 9, 2021
  31. Derrick StoleeFeb 9, 2021
  32. Elijah NewrenFeb 9, 2021
  33. 4/4 gitdiffcore doc: mention new preliminary step for rename detectionElijah Newren via GitGitGadget, Feb 9, 2021
  34. Derrick StoleeFeb 9, 2021
  35. Junio C HamanoFeb 9, 2021
  36. Elijah NewrenFeb 9, 2021
  37. 0/5 Optimization batch 7: use file basenames to guide rename detectionElijah Newren via GitGitGadget, Feb 10, 2021
  38. 1/5 t4001: add a test comparing basename similarity and content similarityElijah Newren via GitGitGadget, Feb 10, 2021
  39. Junio C HamanoFeb 13, 2021
  40. Elijah NewrenFeb 13, 2021
  41. Junio C HamanoFeb 13, 2021
  42. Elijah NewrenFeb 14, 2021
  43. Junio C HamanoFeb 14, 2021
  44. Elijah NewrenFeb 14, 2021
  45. 2/5 diffcore-rename: compute basenames of all source and dest candidatesElijah Newren via GitGitGadget, Feb 10, 2021
  46. Junio C HamanoFeb 13, 2021
  47. 4/5 diffcore-rename: guide inexact rename detection based on basenamesElijah Newren via GitGitGadget, Feb 10, 2021
  48. Junio C HamanoFeb 13, 2021
  49. 3/5 diffcore-rename: complete find_basename_matches()Elijah Newren via GitGitGadget, Feb 10, 2021
  50. Junio C HamanoFeb 13, 2021
  51. Elijah NewrenFeb 13, 2021
  52. Junio C HamanoFeb 13, 2021
  53. Elijah NewrenFeb 14, 2021
  54. 5/5 gitdiffcore doc: mention new preliminary step for rename detectionElijah Newren via GitGitGadget, Feb 10, 2021
  55. Junio C HamanoFeb 10, 2021
  56. Elijah NewrenFeb 10, 2021
  57. 0/6 Optimization batch 7: use file basenames to guide rename detectionElijah Newren via GitGitGadget, Feb 11, 2021
  58. 1/6 t4001: add a test comparing basename similarity and content similarityElijah Newren via GitGitGadget, Feb 11, 2021
  59. 2/6 diffcore-rename: compute basenames of all source and dest candidatesElijah Newren via GitGitGadget, Feb 11, 2021
  60. 3/6 diffcore-rename: complete find_basename_matches()Elijah Newren via GitGitGadget, Feb 11, 2021
  61. 5/6 gitdiffcore doc: mention new preliminary step for rename detectionElijah Newren via GitGitGadget, Feb 11, 2021
  62. 4/6 diffcore-rename: guide inexact rename detection based on basenamesElijah Newren via GitGitGadget, Feb 11, 2021
  63. 6/6 merge-ort: call diffcore_rename() directlyElijah Newren via GitGitGadget, Feb 11, 2021
  64. Junio C HamanoFeb 13, 2021
  65. 0/6 Optimization batch 7: use file basenames to guide rename detectionElijah Newren via GitGitGadget, Feb 14, 2021
  66. 3/6 diffcore-rename: complete find_basename_matches()Elijah Newren via GitGitGadget, Feb 14, 2021
  67. 5/6 gitdiffcore doc: mention new preliminary step for rename detectionElijah Newren via GitGitGadget, Feb 14, 2021
  68. 1/6 t4001: add a test comparing basename similarity and content similarityElijah Newren via GitGitGadget, Feb 14, 2021
  69. 2/6 diffcore-rename: compute basenames of source and dest candidatesElijah Newren via GitGitGadget, Feb 14, 2021
  70. 4/6 diffcore-rename: guide inexact rename detection based on basenamesElijah Newren via GitGitGadget, Feb 14, 2021
  71. 6/6 merge-ort: call diffcore_rename() directlyElijah Newren via GitGitGadget, Feb 14, 2021

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.