On 7/20/2018 1:41 PM, Duy Nguyen wrote:
On Fri, Jul 20, 2018 at 6:35 PM Derrick Stolee <dstolee@xxxxxxxxxxxxx> wrote:
There are many places in Git that use a commit walk to determine
reachability between commits and/or refs. A lot of this logic is
duplicated.
I wanted to achieve the following:
Consolidate several different commit walks into one file
I'm surprised get_shallow_commits() in shallow.c didn't make the cut.
It's no problem though if you already considered it and decided it was
better left alone.
Thanks for pointing this out. I didn't know about it. It would make an
excellent follow-up series.
Reduce duplicate reachability logic
Increase testability (correctness and performance)
Improve performance of reachability queries
What's your recommendation on adding new commit reachability code? I
might have to add one to fix prune_shallow() if I don't find anything
fit. I guess the code should go to commit-reach.c too?
In my opinion, new commit walks should go into commit-reach.c. Then, you
can justify why you are using a "new" walk instead of using an existing
walk. Further, you can probably think of the walk in more generic terms
than the specific application you need. Finally, you can use the
'test-tool reach <method>' pattern to test the specific walk you create
outside of the logic for which you needed it.
I understand that while this patch is under review, you will probably
want to continue adding your walk where it is, then we can consolidate
the code after both have settled.
Thanks,
-Stolee
