Re: [PATCH v2 00/18] Consolidate reachability logic

2018-08-01 Thread Derrick Stolee
On 7/20/2018 6:16 PM, Stefan Beller wrote: * Use single rev-parse commands in test output, and pipe the OIDs through 'sort' Why do we need to sort them? The order of the answers given by rev-parse is the same as the input given and we did not need to sort it before, i.e. the unit under test

Re: [PATCH v2 00/18] Consolidate reachability logic

2018-07-20 Thread Junio C Hamano
Derrick Stolee writes: > 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 > Reduce duplicate

Re: [PATCH v2 00/18] Consolidate reachability logic

2018-07-20 Thread Stefan Beller
Hi Derrick, > Sure! It's on my fork [1] > > [1] https://github.com/derrickstolee/git/tree/reach/refactor > Thanks! > >> * Use single rev-parse commands in test output, and pipe the OIDs through > >> 'sort' Why do we need to sort them? The order of the answers given by rev-parse is the same as

Re: [PATCH v2 00/18] Consolidate reachability logic

2018-07-20 Thread Derrick Stolee
On 7/20/2018 2:09 PM, Eric Sunshine wrote: On Fri, Jul 20, 2018 at 1:20 PM Derrick Stolee wrote: Here is the diff between v1 and v2. diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh @@ -67,142 +67,176 @@ test_three_modes () { test_expect_success 'get_merge_bases_many' '

Re: [PATCH v2 00/18] Consolidate reachability logic

2018-07-20 Thread Derrick Stolee
On 7/20/2018 1:41 PM, Duy Nguyen wrote: On Fri, Jul 20, 2018 at 6:35 PM Derrick Stolee 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

Re: [PATCH v2 00/18] Consolidate reachability logic

2018-07-20 Thread Eric Sunshine
On Fri, Jul 20, 2018 at 1:20 PM Derrick Stolee wrote: > Here is the diff between v1 and v2. > > diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh > @@ -67,142 +67,176 @@ test_three_modes () { > test_expect_success 'get_merge_bases_many' ' > cat >input <<-\EOF && > +

Re: [PATCH v2 00/18] Consolidate reachability logic

2018-07-20 Thread Duy Nguyen
On Fri, Jul 20, 2018 at 6:35 PM Derrick Stolee 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

Re: [PATCH v2 00/18] Consolidate reachability logic

2018-07-20 Thread Derrick Stolee
Here is the diff between v1 and v2. Thanks, -Stolee --- diff --git a/bisect.c b/bisect.c index e1275ba79e..d023543c91 100644 --- a/bisect.c +++ b/bisect.c @@ -13,6 +13,7 @@ #include "sha1-array.h" #include "argv-array.h" #include "commit-slab.h" +#include "commit-reach.h" static struct

Re: [PATCH v2 00/18] Consolidate reachability logic

2018-07-20 Thread Derrick Stolee
On 7/20/2018 1:10 PM, Stefan Beller wrote: Hi Derrick, V2 Update: The biggest material change in this version is that we drop the method declarations from commit.h, which requires adding a lot of references to commit-reach.h across the codebase. This change is in a commit on its own. In

Re: [PATCH v2 00/18] Consolidate reachability logic

2018-07-20 Thread Stefan Beller
Hi Derrick, > V2 Update: The biggest material change in this version is that we drop the > method declarations from commit.h, which requires adding a lot of references > to commit-reach.h across the codebase. This change is in a commit on its own. > In addition, we have the following smaller

[PATCH v2 00/18] Consolidate reachability logic

2018-07-20 Thread Derrick Stolee
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 Reduce duplicate reachability logic Increase testability